Skip to content

fix: Fix path mismatches on Windows #2414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 13, 2025
Merged

fix: Fix path mismatches on Windows #2414

merged 4 commits into from
Mar 13, 2025

Conversation

BYK
Copy link
Member

@BYK BYK commented Mar 12, 2025

Attempts to fix #2413

@BYK BYK requested review from mitsuhiko and loewenheim March 12, 2025 20:12
Comment on lines +497 to +502
// TODO: Enable the following test and make it pass
// Linux allows having `\` in a file name but our path helpers,
// specifically `join_path` and `clean_path` from `symbolic::common`,
// do not use `std::path::MAIN_SEPARATOR` and instead tries to guess
// the path style by the existence of a `\` in the path. This is
// problematic because it can lead to incorrect path normalization.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to explain this a bit: the functions in symbolic work this way because they are not necessarily intended to be used on paths existing on the machine they're running on. Rather, they are used for paths stored in debug files and the like, which we want to combine and canonicalize. For this purpose they should work independently of the host system. For example, a path stored in a Windows debug file should be transformed and displayed the same regardless of whether you're using symbolic on Windows or Unix.

Copy link
Member

Choose a reason for hiding this comment

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

Overall I think we could also state that backslashes in Unix paths are not supported by sentry-cli and leave it at that, it should be extremely uncommon for backslashes to be present anyway.


assert_eq!(
normalize_sourcemap_url(
&format!("foo{0}bar{0}baz.js", std::path::MAIN_SEPARATOR),
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about this syntax

Copy link
Member Author

Choose a reason for hiding this comment

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

Me too. This is my first working Rust patch 😂

@lcian lcian merged commit c032825 into master Mar 13, 2025
14 checks passed
@lcian lcian deleted the byk/fix/windows-paths branch March 13, 2025 10:07
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.

Incorrect handling of path separators under Windows
3 participants