Repository navigation
refuse a ':' in a path a mount point serves - #2643
Merged
Merged
Conversation
On NTFS a ':' in a file name selects a stream of the file, so "/a.txt::$DATA" serves a.txt past a pre-routing check on "/a.txt", and "/a.txt:name" serves a named stream such as Zone.Identifier. is_valid_path now refuses ':' on every platform, as it does a segment ending in '.' or a space.
An NTFS stream name is the only thing ':' means in a path, and only on Windows. Elsewhere it is an ordinary file name character, so a file such as 2024-01-01T10:00:00.log would stop being served. The test pins both sides.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
b4b2458 made
is_valid_pathrefuse a segment ending in.or a space because Windows maps those names onto another file, but:still gets through, and on NTFS it does the same job by naming a stream of the file. A request for/dir/test.html::$DATAopens the default stream ofdir/test.html, so aset_pre_routing_handler()guard on/dir/test.htmlis passed and the file is served anyway, while/setup.exe:Zone.Identifierserves the stream Windows attaches to downloaded files, which records the URL they came from._fullpathkeeps the stream suffix, so the containment check against the resolved base still passes, and as far as I can tell the UCRT_wstatandCreateFile2both open the stream (MinGW's_wstatmay refuse it). I noticed it while reading the alias cases the README still lists, where stream names are not mentioned. Refusing:inis_valid_pathcloses it, and I have applied it on every platform to match the dot and space rule, so the mapping fromreq.pathto a file stays the same wherever the server runs. The cost should be small, though a POSIX deployment serving file names with a colon would now get a 404 for them.Reviewer note. I could only run the macOS no-TLS and split builds:
IsValidPathTest.RejectsStreamNamesfails before the change and passes after, while the two newMountTest.AliasedPathDoesNotPassPreRoutingGuardcases only reach the stream path on the Windows (MSVC) job, so I have not observed that half directly. If you would rather keep:on POSIX, scoping the check to_WIN32is a one-line change.