Skip to content

Conversation

@polymood
Copy link

Fixes #717

On the Windows operating system, downloads would fail for any file containing a colon (:) in its name. This was caused by a two-part bug that manifested in either a ValueError or a WinError 3.

Root Cause:

  1. The utils.sanitize_filepath function used os.path.basename, which incorrectly interpreted a colon in a simple filename as a drive separator on Windows. This caused the sanitization to fail silently.
  2. This incorrect, unsanitized filename was then passed to the files.download method. The security check in this method would correctly reject the malformed path, leading to the crash.

This PR introduces a comprehensive fix:

  1. utils.py: The sanitize_filepath function has been completely rewritten to be more robust. It now correctly handles simple filenames by first checking for the absence of path separators, and uses rsplit() to reliably parse full paths. This ensures correct behavior on all platforms without relying on the problematic os.path functions.

  2. files.py: The security check within the download method has been refactored to a more explicit and reliable pathlib pattern. It now establishes a trusted absolute base directory first, then safely joins the sanitized filename before validation. This change improves security and prevents subtle path resolution bugs.

  3. tests/test_utils.py:

    • New unit tests have been added to specifically target the Windows colon bug and prevent future regressions.
    • An existing brittle test (test_get_file_size) has been fixed by updating a hardcoded value.
    • The existing test_sanitize_filepath test has been made platform-aware using mocks to ensure it passes on both Windows and POSIX systems.

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.

Bug: [Windows] Filenames with colons cause download failure (ValueError / WinError 3)

1 participant