fix(docparser): prevent exception escape in doConvertFile from aborti…#62
Merged
Merged
Conversation
…ng host process
- Move createParser() call and nullptr check inside the try block in doConvertFile() so parser constructor exceptions are
also caught
- Replace throwing std::logic_error with a log + return {} for unsupported extensions, aligning with
tryConvertWithTruncation()
- Prevents std::logic_error from escaping DocParser::convertFile, which previously triggered std::terminate -> abort in
callers without try-catch (e.g. dde-file-manager TextExtractor::extract)
- Add binary test file test_crash_unsupported_ext.dat to reproduce/regress the unsupported-extension crash path
修复(docparser): 防止 doConvertFile 中异常逃逸导致宿主进程崩溃
- 将 createParser() 调用和 nullptr 检查移入 doConvertFile() 的 try 块内，确保解析器构造期异常也被捕获
- 不支持的扩展名由抛出 std::logic_error 改为打印日志并返回空串，与 tryConvertWithTruncation() 写法保持一致
- 防止 std::logic_error 从 DocParser::convertFile 逃逸，原先会在无 try-catch 的调用方（如 dde-file-manager 的
TextExtractor::extract）触发 std::terminate -> abort
- 新增二进制测试文件 test_crash_unsupported_ext.dat，用于复现/回归不支持扩展名的崩溃路径
Log: 修复 doConvertFile 中异常逃逸导致宿主进程 std::terminate -> abort
的问题，将不支持扩展名的处理由抛异常改为返回空串，并把 createParser 调用纳入 try 保护
Task: https://pms.uniontech.com/task-view-391293.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts DocParser::doConvertFile to handle unsupported file extensions and parser construction failures without throwing, ensuring exceptions don’t escape and adding logging instead of logic_error, plus adds a binary test input to cover the unsupported-extension crash path. Sequence diagram for updated DocParser::doConvertFile exception handlingsequenceDiagram
actor HostProcess
participant TextExtractor as TextExtractor::extract
participant DocParser as DocParser::convertFile
participant doConvertFile as doConvertFile
participant Factory as createParser
participant Parser as FileExtension
HostProcess ->> TextExtractor: extract(filename)
TextExtractor ->> DocParser: convertFile(filename)
DocParser ->> doConvertFile: doConvertFile(filename, suffix)
activate doConvertFile
doConvertFile ->> Factory: createParser(filename, suffix)
alt Unsupported_extension
Factory -->> doConvertFile: nullptr
doConvertFile ->> doConvertFile: std::cout << "Unsupported file extension"
doConvertFile -->> DocParser: return {}
DocParser -->> TextExtractor: return {}
else Supported_extension
Factory -->> doConvertFile: Parser instance
doConvertFile ->> Parser: convert()
alt [exception in createParser or convert]
doConvertFile -->> DocParser: return {}
DocParser -->> TextExtractor: return {}
else [no exception]
Parser -->> doConvertFile: m_text
doConvertFile -->> DocParser: return m_text
DocParser -->> TextExtractor: return m_text
end
end
deactivate doConvertFile
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Using
std::coutfor the unsupported extension path may not integrate with existing logging/diagnostic mechanisms; consider routing this through the project’s logging facility instead. - Changing unsupported extensions from throwing to returning
{}now makes it indistinguishable from a successfully parsed but empty document; if callers need to differentiate these cases, consider returning an explicit error indicator or status alongside the string.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `std::cout` for the unsupported extension path may not integrate with existing logging/diagnostic mechanisms; consider routing this through the project’s logging facility instead.
- Changing unsupported extensions from throwing to returning `{}` now makes it indistinguishable from a successfully parsed but empty document; if callers need to differentiate these cases, consider returning an explicit error indicator or status alongside the string.
## Individual Comments
### Comment 1
<location path="src/docparser.cpp" line_range="294" />
<code_context>
try {
+ std::unique_ptr<fileext::FileExtension> document = createParser(filename, suffix);
+ if (!document) {
+ std::cout << "Unsupported file extension: " << filename << std::endl;
+ return {};
+ }
</code_context>
<issue_to_address>
**suggestion:** Avoid writing directly to std::cout in a lower-level conversion helper; prefer a logging facility or std::cerr, or make logging configurable.
This ties a low-level parsing routine to user-facing stdout, which can be ignored or reserved for structured output in GUIs, services, or libraries. Prefer a logging facility, std::cerr, or delegating logging to the caller, potentially behind a debug/verbose flag, so normal use doesn’t emit unexpected console output.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| try { | ||
| std::unique_ptr<fileext::FileExtension> document = createParser(filename, suffix); | ||
| if (!document) { | ||
| std::cout << "Unsupported file extension: " << filename << std::endl; |
There was a problem hiding this comment.
suggestion: Avoid writing directly to std::cout in a lower-level conversion helper; prefer a logging facility or std::cerr, or make logging configurable.
This ties a low-level parsing routine to user-facing stdout, which can be ignored or reserved for structured output in GUIs, services, or libraries. Prefer a logging facility, std::cerr, or delegating logging to the caller, potentially behind a debug/verbose flag, so normal use doesn’t emit unexpected console output.
deepin pr auto review☀
|
Contributor
Author
不是qt库，默认采用std输出 |
max-lvs
approved these changes
Jun 22, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, pppanghu77 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ng host process
修复(docparser): 防止 doConvertFile 中异常逃逸导致宿主进程崩溃
Log: 修复 doConvertFile 中异常逃逸导致宿主进程 std::terminate -> abort
的问题，将不支持扩展名的处理由抛异常改为返回空串，并把 createParser 调用纳入 try 保护
Task: https://pms.uniontech.com/task-view-391293.html
Summary by Sourcery
Handle unsupported document file extensions and parser construction errors in DocParser::doConvertFile without crashing the host process.
Bug Fixes: