Read Microsoft FTP Service listings - #78
Open
christhomas wants to merge 1 commit into
Open
Conversation
Fixes secsy#43, secsy#54 and secsy#65 — the same cause reported three times. IIS does not send an ls-style listing: 10-09-20 09:36PM <DIR> aspnet_client 10-16-20 05:20PM 6989 Biography.html 03-02-2023 03:15PM <DIR> Archived Tracking No permission bits, no owner, no link count. The unix regex cannot match any of it, so ReadDir failed outright on every IIS server — one reporter had it panic. Three details that are easy to get wrong, all covered by tests taken from the reports: The year is two digits or four, in listings from the same server family. The layout follows the year's width rather than a pivot of our own, because time.Parse already decides what a two-digit year means and disagreeing with it here would be a second rule to maintain. A directory is marked <DIR> where a size would be. It gets no size, rather than zero dressed up as one — the server did not send a number. The name runs to the end of the line, so it may contain spaces: "Archived Tracking" is one entry, not two. The unix format is tried first and is unchanged, so this adds a format rather than replacing one. Both are still rejected for a line that is neither, including a DOS-shaped line with an impossible time — the looser pattern must not swallow what the stricter one refused.
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
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 #43, #54 and #65 — the same cause, reported three times over four years.
IIS does not send an ls-style listing:
No permission bits, no owner, no link count.
lsRegexmatches none of it, soReadDirfails outright on every IIS server — @dorimon-1 reported it as a panic in #65.Three details that are easy to get wrong
10-09-20in Microsoft FTP Service #54,03-02-2023in ReadDir() cannot read windows hosted ftp directories #65). The layout follows the year's width rather than a pivot of our own:time.Parsealready decides what a two-digit year means, and disagreeing with it here would be a second rule to maintain.<DIR>where a size would be. It gets no size, rather than zero dressed up as one — the server did not send a number.Archived Trackingis one entry, not two.Every test entry is copied from one of the three reports, plus midnight and noon, which is where 12-hour clocks go wrong.
What it does not change
lsRegexis tried first and is untouched, so this adds a format rather than replacing one.parseDOSLISTreturns(nil, nil)when the line is not in its format, so the caller reports the failure it was already going to report rather than replacing it with a less accurate one.A line that is neither format is still rejected — including a DOS-shaped line with an impossible time, because the looser pattern must not swallow what the stricter one refused. That case is tested.
@dorimon-1 offered a PR for this in #65; happy to defer if that lands first.
One note on running the tests: this branch is off
master, soTestMainstill requires./build_test_server.sh— these tests need no server, but they cannot start without one. I verified them via the container harness in #73.