Skip to content

fix: prevent crash saving PDF to unwritable dir - #429

Closed
LiHua000 wants to merge 2 commits into
linuxdeepin:masterfrom
LiHua000:agent/agent/8708bd90da6b
Closed

LiHua000 wants to merge 2 commits into
linuxdeepin:masterfrom
LiHua000:agent/agent/8708bd90da6b

Conversation

@LiHua000

@LiHua000 LiHua000 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Root Cause Analysis

DTK DPrintPreviewDialogPrivate::_q_startPrint sets the printer output file path via printer->setOutputFileName(str) without checking directory write permission. When the target directory is not writable, DPrintPreviewWidget::asynPrint() constructs QPainter painter(previewPrinter) which fails (QPrinter cannot open the file, print engine is null). The subsequent painter.setClipRect() dereferences the null engine pointer at offset 0x1c, causing SIGSEGV. Core dump signal signo=11, si_addr=0x1c confirms this path.

Key evidence: dprintpreviewdialog.cpp:2504 (setOutputFileName without permission check), dprintpreviewwidget.cpp:215 (QPainter construction on unwritable QPrinter), dprintpreviewwidget.cpp:221 (setClipRect dereference crash point).

Fix

Added directory writability check at the start of both DocSheet::onPrintRequested overloads using QTemporaryFile probe. When the directory is not writable, a DDialog warning is shown and the output path is redirected to QStandardPaths::TempLocation. This ensures QPainter construction succeeds, preventing the crash. Normal writable paths have no behavior change.

The root cause fix (DTK-side permission check before setOutputFileName) was not feasible because the dtkwidget repository is not available in the current workspace. Subclassing/event-filtering DPrintPreviewDialog was also not feasible (_q_startPrint is a private non-virtual slot with no interceptable signal between file dialog return and pview->print()). The paintRequested signal callback was used instead, as it fires after outputFileName() is set but before the crash path.

Change Safety Assessment

Code Safety

  • Risk Level: Low
  • Only adds permission check at function entry; no signature change, no existing line modification, no historical fix reverted
  • Both onPrintRequested overloads are connected via paintRequested signal in onPopPrintDialog() — connection relationships unchanged

Business Impact Scope

  • Print Preview → Save as PDF: When user selects an unwritable directory, a dialog appears and file is redirected to temp directory instead of crashing
  • Normal printing to physical printer: Unaffected (outputFileName() is empty, permission check is skipped)
  • Save PDF to writable directory: Unaffected (permission check passes, original flow continues)

Verification Suggestion

  1. Save PDF to a writable directory (e.g., Desktop) — confirm no behavior change
  2. Save PDF to an unwritable directory (e.g., /root) — confirm dialog appears and no crash
  3. Print to a physical printer — confirm unaffected

根因分析

DTK DPrintPreviewDialogPrivate::_q_startPrint 在用户选择保存路径后直接调用 printer->setOutputFileName(str) 而不检查目录写权限。当目录不可写时,DPrintPreviewWidget::asynPrint() 中 QPainter painter(previewPrinter) 构造失败(QPrinter 无法打开文件,打印引擎为空),随后 painter.setClipRect() 解引用空引擎指针(偏移 0x1c),导致 SIGSEGV。core dump 信号 signo=11, si_addr=0x1c 与此路径吻合。

关键证据:dprintpreviewdialog.cpp:2504(setOutputFileName 无权限检查)、dprintpreviewwidget.cpp:215(QPrinter 不可写时 QPainter 构造失败)、dprintpreviewwidget.cpp:221(setClipRect 解引用空指针崩溃点)。

修复方案

在 DocSheet::onPrintRequested 两个重载函数开头,用 QTemporaryFile 探测输出目录写权限。不可写时弹出 DDialog 提示并重定向到 QStandardPaths::TempLocation,使 QPainter 构造成功,避免崩溃。正常可写路径无行为变化。

根因层修复(DTK 侧 setOutputFileName 前加权限检查)因 dtkwidget 仓库不在当前工作区不可行。子类化/事件过滤 DPrintPreviewDialog 也不可行(_q_startPrint 为私有非虚槽,文件对话框返回与 pview->print() 之间无可拦截信号)。实际改用 paintRequested 信号回调中检查写权限并重定向。

改动安全评估

代码安全评估

  • 风险等级: 低风险
  • 仅在函数入口添加权限检查,不修改函数签名、不修改已有行、不撤销历史修复
  • 两个 onPrintRequested 重载由 onPopPrintDialog() 中 paintRequested 信号连接,连接关系不变

业务影响范围

  • 打印预览保存为 PDF:用户选择不可写目录时弹出提示并重定向到临时目录,不再崩溃
  • 正常打印到物理打印机:不受影响(outputFileName() 为空,权限检查跳过)
  • 保存 PDF 到可写目录:不受影响(权限检查通过,继续原流程)

验证建议

  1. 保存 PDF 到可写目录(如桌面),确认行为无变化
  2. 保存 PDF 到不可写目录(如 /root),确认弹出提示对话框且不崩溃
  3. 正常打印到物理打印机,确认不受影响

PMS: BUG-380945

Summary by Sourcery

Prevent unwritable PDF export locations from crashing the application and add localized Document Viewer documentation.

Bug Fixes:

  • Prevent crashes when saving PDFs to directories without write permission by notifying the user and redirecting output to a temporary directory.

Documentation:

  • Add Document Viewer user guides in English, Simplified Chinese, Traditional Chinese, and Hong Kong Chinese.

LiHua000 and others added 2 commits June 29, 2026 13:27
1. Root cause: DTK DPrintPreviewDialog sets printer output file
   path without checking directory write permission, QPainter
   construction on unwritable QPrinter creates null engine,
   setClipRect dereferences null pointer causing SIGSEGV
2. Fix: add directory writability check at start of both
   onPrintRequested overloads using QTemporaryFile probe,
   redirect to temp directory with DDialog warning when unwritable
3. Impact: only affects save-to-PDF with unwritable target
   directory, normal writable paths have no behavior change

Log: Fix crash when saving PDF to directory without write permission

Influence:
1. Test save PDF to writable directory (normal path)
2. Test save PDF to unwritable directory (should show dialog, no crash)
3. Test normal printing to physical printer (unaffected)

fix: 修复保存PDF到无权限目录时崩溃的问题

1. 根因:DTK DPrintPreviewDialog 设置打印输出文件路径时未检查
   目录写权限,QPrinter 在不可写路径上构造 QPainter 时引擎为空,
   setClipRect 解引用空指针导致 SIGSEGV
2. 方案:在两个 onPrintRequested 重载开头用 QTemporaryFile 探测
   目录写权限,不可写时弹出 DDialog 提示并重定向到临时目录
3. 影响:仅影响保存PDF到不可写目录的场景,正常可写路径无行为变化

Log: 修复保存PDF到无权限目录时崩溃的问题

Influence:
1. 测试保存PDF到可写目录(正常路径)
2. 测试保存PDF到不可写目录(应弹出提示,不崩溃)
3. 测试正常打印到物理打印机(不受影响)

PMS: BUG-380945
@sourcery-ai

sourcery-ai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Sorry @LiHua000, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 3 days and 3 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000

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

@sourcery-ai

sourcery-ai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR prevents PDF-printing crashes caused by unwritable output directories by probing directory access before painting and redirecting failures to a temporary location, while also adding localized Document Viewer user guides.

Sequence diagram for safe PDF printing to an unwritable directory

sequenceDiagram
    actor User
    participant DocSheet
    participant QTemporaryFile
    participant DDialog
    participant QPrinter
    participant QPainter
    participant TempLocation

    User->>DocSheet: onPrintRequested(printer)
    DocSheet->>QPrinter: outputFileName()
    alt output path is unwritable
        DocSheet->>QTemporaryFile: open()
        QTemporaryFile-->>DocSheet: failure
        DocSheet->>DDialog: exec()
        DDialog-->>DocSheet: accepted
        DocSheet->>TempLocation: writableLocation(TempLocation)
        TempLocation-->>DocSheet: temporary directory
        DocSheet->>QPrinter: setOutputFileName(temp path)
    else output path is writable or empty
        DocSheet->>QTemporaryFile: open()
        QTemporaryFile-->>DocSheet: success or skipped
    end
    DocSheet->>QPainter: QPainter(printer)
    QPainter-->>DocSheet: painter constructed without crash
Loading

File-Level Changes

Change Details Files
Add a pre-print output-directory writability probe and redirect failed PDF destinations to the temporary directory.
  • Check non-empty printer output paths with a QTemporaryFile probe in both print-request handlers.
  • Show a warning dialog and log the failure when the selected directory cannot accept a temporary file.
  • Replace the printer output path with the same filename under QStandardPaths::TempLocation before constructing the painter.
reader/uiframe/DocSheet.cpp
Add user-facing Document Viewer help content for supported locales.
  • Add English, Simplified Chinese, Traditional Chinese, and Hong Kong Chinese usage guides covering startup, document operations, printing, annotations, navigation, and menus.
  • Reference existing localized icons and screenshots from the help content.
assets/deepin-reader/document-viewer/en_US/document-viewer.md
assets/deepin-reader/document-viewer/zh_CN/document-viewer.md
assets/deepin-reader/document-viewer/zh_HK/document-viewer.md
assets/deepin-reader/document-viewer/zh_TW/document-viewer.md

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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 97 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交修复了PDF保存到不可写目录时的崩溃问题,通过检测输出目录写权限并在不可写时重定向到临时目录来防止QPainter构造时崩溃。代码逻辑正确,安全无漏洞,但存在代码重复问题,建议提取为公共方法。同时新增了4份多语言用户手册文档。

🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: []


2. 代码质量 ✅

评价: 代码结构清晰,注释完整 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['将重复的权限检查和重定向逻辑提取为辅助方法,例如:void DocSheet::ensureOutputDirWritable(DPrinter *printer),在两个 onPrintRequested 重载中调用', '简化字符串拼接:将 QString("dr_") + "exception-logo" 改为 QStringLiteral("dr_exception-logo")']


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: []


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: []


💡 改进建议代码示例

// 建议提取公共辅助方法,消除代码重复: // // void DocSheet::ensureOutputDirWritable(DPrinter *printer) // { //     QString outputPath = printer->outputFileName(); //     if (outputPath.isEmpty()) //         return; // //     QString dir = QFileInfo(outputPath).absolutePath(); //     QTemporaryFile testFile(dir + "/.dr_print_test_XXXXXX"); //     if (testFile.open()) //         return; // //     qCWarning(appLog) << "Output directory not writable:" << dir //                       << ", redirecting to temp"; //     DDialog dlg("", tr("The selected directory does not have write permission. " //                        "The file will be saved to the temporary directory.")); //     dlg.setIcon(QIcon::fromTheme(QStringLiteral("dr_exception-logo"))); //     dlg.addButtons(QStringList() << tr("OK", "button")); //     QMargins mar(0, 0, 0, 30); //     dlg.setContentLayoutContentsMargins(mar); //     dlg.exec(); // //     QString tempDir = QStandardPaths::writableLocation(QStandardPaths::TempLocation); //     printer->setOutputFileName(tempDir + "/" + QFileInfo(outputPath).fileName()); // }

本报告由 AI 代码审查工具自动生成

@LiHua000 LiHua000 closed this Oct 28, 2026
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