tool_xattr: add support for Windows alternate data stream - #22354
tool_xattr: add support for Windows alternate data stream#22354vszakats wants to merge 1 commit into
Conversation
|
As normal with Windows, even if a thing looks simple, it isn't: |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Pull request overview
Adds Windows support for --xattr by writing Mark-of-the-Web metadata into an NTFS Alternate Data Stream (Zone.Identifier), and updates existing xattr tests to validate referer/origin metadata (with Win32-specific expectations).
Changes:
- Implement Win32
--xattrhandling by writing aZone.IdentifierADS alongside downloaded files. - Extend
fwrite_xattr()to accept the output filename and wire the new signature into the transfer pipeline. - Update xattr-related tests to include
--refererand to verify Win32 vs non-Win32 metadata output formats.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/data/test644 | Adjust expected xattr output for Win32 (ZoneTransfer) vs non-Win32 (user.* xattrs). |
| tests/data/test687 | Adjust expected xattr output for Win32 (ZoneTransfer) vs non-Win32 (user.* xattrs). |
| tests/data/test688 | Add --referer and verify referer propagation; add Win32 ZoneTransfer expectations. |
| src/tool_xattr.h | Enable xattr support on Win32; update fwrite_xattr() signature. |
| src/tool_xattr.c | Implement Win32 ADS writing of ZoneTransfer metadata; keep non-Win32 xattr behavior. |
| src/tool_operate.c | Pass output filename into fwrite_xattr() to support Win32 ADS creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Environment variables can be inherited which makes them sometimes pernicious. Generally I'm not a fan of curl environment variables except for debug stuff. Global states can in some cases break scripts which could force scripts to blank it beforehand ( I think an option like
This is tricky because in Windows a colon comes after the drive letter and before alternate data streams. So |
|
I also wouldn't say I'm a big fan of envs, but for a feature like this, I'm (mildly put) not (Besides Safari, Homebrew sets this, luckily with still an option in place to disable it.) FWIW a command-line option can also have the same effect by pointing I managed to solve (I think) the I've since renamed the env to If this is controversial or bad, I will just delete this feature from this PR and leave it to |
Refs: https://en.wikipedia.org/wiki/Mark_of_the_Web https://en.wikipedia.org/wiki/NTFS#Alternate_data_stream_(ADS) https://learn.microsoft.com/en-us/windows/win32/fileio/file-streams https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/c54dec26-1551-4d3a-a0ea-4fa40f848eb3 Ref: curl#22345 temp add debug output t: try adding windows support test688 test referrer fix newlines move referrer above url to match non-Windows fixups rename-var convert to absolute path comment update docs pyspelling
Requires a supported filesystem, i.e. NTFS.
Example:
Where newlines are CLRF,
ReferrerUrlis set when using--refereroption.
Also:
--referer.user.xdg.referrer.urlin--xattrman page.Refs:
https://en.wikipedia.org/wiki/Mark_of_the_Web
https://en.wikipedia.org/wiki/NTFS#Alternate_data_stream_(ADS)
https://learn.microsoft.com/en-us/windows/win32/fileio/file-streams
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/c54dec26-1551-4d3a-a0ea-4fa40f848eb3
Ref: #22345
At least reject them, possibly expand to full path before fopen?