Skip to content

fix(high_level): Enhance the reliability of temporary file cleanup#721

Merged
awwaawwa merged 2 commits intoPDFMathTranslate:mainfrom
lintian233:tmpfix
Mar 4, 2025
Merged

fix(high_level): Enhance the reliability of temporary file cleanup#721
awwaawwa merged 2 commits intoPDFMathTranslate:mainfrom
lintian233:tmpfix

Conversation

@lintian233
Copy link
Contributor

原代码问题

  • 路径误判风险:startswith可能匹配到/tmp/../etc/passwd类路径
  • Windows 短路径名无法识别 (如C:\Users\~1\...)
  • 无错误处理:文件被占用时静默失败

修改

  • pathlib.Path.samefile()`替代字符串匹配
  • 添加捕获异常并提示具体路径

@awwaawwa awwaawwa requested review from awwaawwa and Copilot March 4, 2025 07:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR enhances the reliability of temporary file cleanup by eliminating unsafe string matching and adding robust error handling.

  • Replaces the vulnerable startswith check with pathlib.Path.samefile for file path verification.
  • Adds exception handling to log a message and prompt manual deletion when cleanup fails.

Reviewed Changes

File Description
pdf2zh/high_level.py Improves temporary file cleanup reliability

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

pdf2zh/high_level.py:371

  • Consider handling cases where a temporary file might reside in a subdirectory of the system temp directory. If such cases are expected, comparing file_path.parent with temp_dir may be insufficient; instead, verify that the resolved file path is within the temp directory's hierarchy.
if file_path.exists() and file_path.parent.samefile(temp_dir):

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR enhances the reliability of temporary file cleanup by replacing string-based path matching with pathlib's functionality and adding error handling.

  • Replace string matching with Path methods (including is_relative_to)
  • Add try/except for file deletion failures and log errors appropriately

Reviewed Changes

File Description
pdf2zh/high_level.py Update temporary file removal to use pathlib and add logging for errors

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

pdf2zh/high_level.py:369

  • [nitpick] Consider renaming the variable 'file' to a more descriptive name (e.g., 'temp_file_path_str') to avoid shadowing the built-in and improve clarity.
file_path = Path(file)

@awwaawwa awwaawwa changed the title fix(pdfzh/high_level) : 增强临时文件清理可靠性 fix(high_level): Enhance the reliability of temporary file cleanup Mar 4, 2025
@awwaawwa awwaawwa merged commit cf74284 into PDFMathTranslate:main Mar 4, 2025
7 checks passed
@awwaawwa
Copy link
Collaborator

awwaawwa commented Mar 4, 2025

Thank you for your contribution. According to https://funstory-ai.github.io/BabelDOC/CONTRIBUTOR_REWARD/ , you can apply for a monthly membership redemption code for Immersive Translate.

@lintian233 lintian233 deleted the tmpfix branch March 4, 2025 10:17
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.

3 participants