Skip to content

fix(album): restore empty album import interaction - #776

Closed
wyu71 wants to merge 0 commit into
linuxdeepin:masterfrom
wyu71:master
Closed

fix(album): restore empty album import interaction#776
wyu71 wants to merge 0 commit into
linuxdeepin:masterfrom
wyu71:master

Conversation

@wyu71

@wyu71 wyu71 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Keep the empty album import area interactive without covering its controls.

修复空专辑页面导入区域覆盖控件导致无法点击的问题。

Log: 修复专辑空页面导入
PMS: BUG-374225
Influence: 空专辑页面可以正常点击导入并支持拖入歌曲。

Summary by Sourcery

Restore empty album import interactions while safely filtering dropped file URLs.

New Features:

  • Enable importing supported audio files into empty albums by dropping them onto the album page.

Bug Fixes:

  • Restore click and drag-and-drop interaction for the empty album import area.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @wyu71, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR restores interactivity of the empty album import area by adding a DropArea over the album default page for drag-and-drop imports and adjusting z-ordering so the import UI remains clickable and visible.

Sequence diagram for empty album drag-and-drop import interaction

sequenceDiagram
    actor User
    participant AlbumDefaultPage
    participant DropArea
    participant Presenter

    User->>AlbumDefaultPage: drag files over empty album area
    AlbumDefaultPage->>DropArea: detect drop event
    DropArea->>DropArea: onDropped(drop)
    DropArea->>Presenter: importMetas(list, album)
    Presenter-->>User: album imports processed
Loading

File-Level Changes

Change Details Files
Enable drag-and-drop importing on the empty album page while preserving access to its controls.
  • Add a DropArea that fills the AlbumDefaultPage to handle dropped URLs and call Presenter.importMetas with type "album".
  • Iterate over drop.urls to build a list of file paths for import.
src/music-player/albumlist/AlbumDefaultPage.qml
Ensure the empty album default UI stays visually on top and clickable when the page is shown.
  • Set z: 1 on the main Rectangle in AlbumDefaultPage so it renders above the background DropArea.
  • Set z: 1 on AlbumDefaultPage in AlbumView to keep the default page above surrounding layout elements.
src/music-player/albumlist/AlbumDefaultPage.qml
src/music-player/albumlist/AlbumView.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@wyu71
wyu71 force-pushed the master branch 4 times, most recently from 1b982c4 to 0542748 Compare August 19, 2026 05:48
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:55分

■ 【总体评价】

代码实现了拖拽导入音频的安全防护,但存在后缀校验绕过和协议黑名单缺陷
逻辑存在路径以斜杠结尾时跳过校验的错误,且安全维度因中危漏洞扣35分

■ 【详细分析】

  • 1.语法逻辑(存在错误)✕
    safeImportUrl函数中,当提取的path/结尾时直接返回原始value,导致后续的文件名提取与后缀白名单校验被完全跳过。
    潜在问题:攻击者可构造以/结尾的恶意URL(如ftp://evil.com/)绕过音频后缀检查;正常目录拖拽也会产生非空返回值被错误传入导入函数。
    建议:移除if (path.endsWith("/")) return value逻辑,统一进行后缀校验,对目录或不合规路径统一返回空字符串。
  • 2.代码质量(良好)✓
    变量命名清晰,safeImportUrl内部处理流程(协议判断、循环解码、路径穿越检测、后缀校验)层次分明,易于理解。
    建议:可将正则表达式提取为组件级常量属性,避免每次调用时重复编译。
  • 3.代码性能(无性能问题)✓
    循环解码decodeURIComponent采用状态机模式,能快速收敛;正则匹配和字符串操作均在可控的短字符串上进行。
    建议:无
  • 4.代码安全(存在 3 个安全漏洞)✕
    漏洞对比统计:新增漏洞 3 个,减少漏洞 0 个,持平 0 个
    整体采用黑名单机制过滤协议且存在校验绕过缺陷,可能导致非预期文件被导入或触发异常处理逻辑。

  • 安全漏洞1(中危):[协议黑名单绕过] 在 [AlbumDefaultPage.qml/safeImportUrl] 中,[输入源为拖拽传入的URL,触发方式为传入未被黑名单拦截的恶意协议(如ftpvbscript等),若配合目录斜杠绕过后缀检查,将导致恶意URL被传入Presenter.importMetas函数,可能引发底层组件的异常解析或SSRF类风险] ——非常重要

  • 安全漏洞2(中危):[白名单校验绕过] 在 [AlbumDefaultPage.qml/safeImportUrl] 中,[输入源为拖拽URL,触发方式为构造以/结尾的路径(如http://evil.com/payload/),代码在path.endsWith("/")判断处直接返回原始值,跳过了isSupportedAudioFile后缀白名单校验,导致任意非音频文件URL可通过检查] ——非常重要

  • 安全漏洞3(低危):[正则表达式注入风险] 在 [AlbumDefaultPage.qml/isSupportedAudioFile] 中,[输入源为supportedAudioSuffixes数组,触发方式为若未来维护时误将未经转义的特殊字符(如(+)加入后缀数组,会导致new RegExp构造出异常或具有ReDoS风险的恶意正则,引发拒绝服务] ——非常重要

  • 建议:将协议过滤从黑名单改为白名单(仅允许file和空协议);删除path.endsWith("/")的特殊返回逻辑,强制要求所有URL必须通过后缀校验;对supportedAudioSuffixes数组元素在拼接正则前进行转义处理。

■ 【改进建议代码示例】

--- a/src/music-player/albumlist/AlbumDefaultPage.qml
+++ b/src/music-player/albumlist/AlbumDefaultPage.qml
@@ -17,43 +17,47 @@ Rectangle {
     property var supportedAudioSuffixes: [
         "aac", "ac3", "aiff", "amr", "ape", "flac", "m4a",
         "mp3", "ogg", "opus", "voc", "wav", "wma"
     ]
+    property var safeSuffixPattern: {
+        var escaped = supportedAudioSuffixes.map(function(s) {
+            return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+        });
+        return new RegExp("\\.(" + escaped.join("|") + ")$", "i");
+    }
     color: Qt.rgba(0, 0, 0, 0)
 
-    function isSupportedAudioFile(fileName) {
-        var suffixPattern = new RegExp("\\.(" + supportedAudioSuffixes.join("|") + ")$", "i")
-        return suffixPattern.test(fileName)
-    }
-
     function safeImportUrl(rawUrl) {
         var value = rawUrl.toString()
         var schemeMatch = value.match(/^([a-z][a-z0-9+.-]*):/i)
         var scheme = schemeMatch ? schemeMatch[1].toLowerCase() : ""
-        if (scheme === "data" || scheme === "javascript" || scheme === "qrc")
+        // 采用白名单机制,仅允许本地文件协议或无协议的相对路径
+        if (scheme !== "" && scheme !== "file")
             return ""
 
         var decodedValue = value
         try {
             var prevValue = ""
             while (prevValue !== decodedValue) {
                 prevValue = decodedValue
                 decodedValue = decodeURIComponent(decodedValue)
             }
         } catch (error) {
             return ""
         }
         if (/(^|\/)\.\.(\/|$)/.test(decodedValue))
             return ""
 
         var path = decodedValue.split(/[?#]/)[0]
-        if (path.endsWith("/"))
-            return value
         var fileName = path.substring(path.lastIndexOf("/") + 1)
-        if (!isSupportedAudioFile(fileName))
+        if (fileName === "" || !safeSuffixPattern.test(fileName))
             return ""
         return value
     }

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wyu71

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants