fix CI with platform-dependent string encoding #125
Conversation
|
cc @Pr0methean as I can't add reviewers (that's fine, I'm not asking for more permissions). |
|
Ok, now I've tried actually using the platform-specific code to extract an |
|
Hmmm, I actually think the change I've added in aef0bb6 is necessary, as master appears to have been broken since 8715d93. Please let me know if I'm mistaken -- I am extremely unfamiliar with handling windows paths and strings and I have no clue whether there's a more canonical approach that I'm missing. |
|
I'm going to make another PR adding |
cosmicexplorer
force-pushed
the
revert-path-join
branch
from
aef0bb6
to
7ed2cdb
Compare
|
Just squashed and force pushed to clarify that no commits were reverted. Will update the description. |
cosmicexplorer
force-pushed
the
revert-path-join
branch
2 times, most recently
from
b551163
to
bfac065
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
cosmicexplorer
changed the title
| /* FIXME: what bug does this code fix? | ||
| * https://github.com/zip-rs/zip2/commit/8715d936cbd3e0fdc2e778b70d8a02e2dfc16fc2 | ||
| * doesn't specify! */ |
There was a problem hiding this comment.
| /* FIXME: what bug does this code fix? | |
| * https://github.com/zip-rs/zip2/commit/8715d936cbd3e0fdc2e778b70d8a02e2dfc16fc2 | |
| * doesn't specify! */ | |
| // Extract symlinks as symlinks |
The bug was that symlinks were extracted as normal files.
| assert!( | ||
| chunks.remainder().is_empty(), | ||
| "windows utf-16 strings should be divisible by 2" | ||
| ); |
There was a problem hiding this comment.
Aren't symlinks stored in UTF-8, given the general principle that file paths are Unix-style? The APPNOTE doesn't specifically address this.
| ); | ||
| let target_wide: Vec<u16> = chunks | ||
| .into_iter() | ||
| .map(|c| u16::from_le_bytes(c.try_into().unwrap())) |
There was a problem hiding this comment.
This will panic if a character code point is above 0xFFFF, because that has to be encoded in UTF-16 as a surrogate pair.
|
From the description of Since UTF-16 isn't a superset of UTF-8, passing UTF-16 to it is wrong. |
|
Closing since https://github.com/zip-rs/zip2/actions/runs/9104341554/job/25028016801 shows that the issue is fixed. |
This fixes the CI failure introduced in 8715d93, as there appears to be no platform-independent way to create an
OsString(see links in docstring forOsString::from_encoded_bytes_unchecked()). This is causing a failure on master.This also fixes an additional compile error in a test file that only shows up with specific features enabled.