Report a symlink's own name from ReadDir, not its target's - #74
Open
christhomas wants to merge 1 commit into
Open
Report a symlink's own name from ReadDir, not its target's#74christhomas wants to merge 1 commit into
christhomas wants to merge 1 commit into
Conversation
Fixes secsy#60. LIST renders a symlink as "name -> target", and the regex captures both as one field. The name was then taken as filepath.Base of the whole thing, which is wrong in two different ways: config -> etc/real.conf reported as "real.conf" link.txt -> lorem.txt reported as "link.txt -> lorem.txt" shortcut -> /var/log/messages reported as "messages" The second is merely malformed. The first and third are the damaging ones: they are plausible filenames, and they are the *target's*, so a caller listing a directory is handed the name of something that is not in it and has no way to tell. The name is now the left side of the first separator. A filename may legitimately contain " -> ", which makes the split ambiguous, but the ambiguity is in LIST's own output — it renders both cases identically, so no reader can do better. Only entries whose mode says symlink are split at all, so a regular file with an arrow in its name is untouched; that case is tested. Verified red before green: the three rows above are what the test printed against the unfixed parser.
Open
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
1 participant
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.
Fixes #60.
LIST renders a symlink as
name -> target, andlsRegexcaptures both as one field.parseLISTthen takes the name asfilepath.Baseof the whole thing.What that produces
Name()todayconfig -> etc/real.confreal.confconfiglink.txt -> lorem.txtlink.txt -> lorem.txtlink.txtshortcut -> /var/log/messagesmessagesshortcutThe second row is merely malformed. The first and third are the damaging ones: they are plausible filenames, and they are the target's — so a caller listing a directory is handed the name of something that is not in that directory, with nothing to indicate it.
The fix
The name is the left side of the first
->, and only for entries whose mode already says symlink — so a regular file with an arrow in its name is untouched. That case is in the test.A filename may legitimately contain
->, which makes the split ambiguous. That ambiguity is in LIST's own output rather than in this code:lsrenders both cases identically, so no reader can distinguish them.Tests
symlink_name_test.go— four entries plus a mode check, no server needed. Written first and confirmed failing on the three rows above before the fix was applied.One note on running them: this branch is off
master, soTestMainstill requires./build_test_server.sh. I verified them via the container harness in #73; with that merged these run anywhere, but the fix itself is independent of it and applies cleanly on its own.