Stat a directory on servers without MLST - #75
Open
christhomas wants to merge 1 commit into
Open
Conversation
Fixes secsy#26. Falling back to "LIST <path>" cannot describe a directory: LIST returns a directory's *contents*, so what came back described something inside it. The count check turned that into an error only when the contents happened not to number one. A directory holding exactly one entry returned that entry, and reported success. Stat("subdir") answered "1234.bin", a regular file, with IsDir() false — which is worse than the reported failure, because nothing about it says it is wrong. Two ways to ask about the entry itself, tried cheapest first. "LIST -d" is ls's flag for "the entry, not what is inside it", and unix-derived servers pass it through; proftpd and pure-ftpd both answer with exactly the one entry. The name in the reply is checked rather than trusted, because a server that ignores the flag lists the contents anyway — and with one entry that is indistinguishable from success by count alone. It is exactly the case that used to go unnoticed. Servers that reject ls flags, IIS among them, need asking differently: list the parent and find the entry by name. That is the fallback the issue suggested. It costs a second round trip, which is why it is second. The root is refused explicitly rather than guessed at — it has no parent to search and no name to match in one. Red before green: both directory tests reported "1234.bin" with IsDir() false against the unfixed code. The fallback is covered by stubbing the "LIST -d" rejection, since neither test server produces it — a fallback nothing exercises is a fallback that does not work. Deliberately not included: the issue also suggests consulting FEAT instead of attempting MLST and handling the failure each time. That is an optimisation rather than a correctness fix, and it belongs in its own change.
christhomas
force-pushed
the
cth/stat-dir-upstream
branch
from
August 26, 2026 13:06
beeb7be to
91fdb55
Compare
This was referenced Aug 26, 2026
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 #26.
The MLST fallback issues
LIST <path>, which cannot describe a directory — LIST returns a directory's contents. Thelen(lines) != 1check turned that into an error only when the contents happened not to number one.What it does today
testroot/subdirholds exactly one file, so:Success, describing a regular file inside the directory. That is worse than the failure the issue reports: nothing about the result indicates it is wrong, so a caller has no way to notice.
The fix, cheapest route first
LIST -d <path>— ls's flag for "the entry, not what is inside it". proftpd and pure-ftpd both pass it through and answer with exactly the one entry.The name in the reply is checked rather than trusted. A server that does not understand the flag lists the contents anyway, and with one entry that is indistinguishable from success by count alone — which is exactly the case that used to go unnoticed.
List the parent and match the basename — for servers that reject ls flags, IIS among them. This is the workaround @gebi suggested in the issue. It is second because it costs a second round trip.
The root is refused explicitly rather than guessed at: it has no parent to search and no name to match in one.
Tests
stat_dir_test.go. Both directory cases were confirmed failing first, reporting1234.binwithIsDir()false.The parent-scan fallback is covered by stubbing the
LIST -drejection, because neither test server produces one — a fallback nothing exercises is a fallback that does not work. The file case passes without the fix and is labelled a regression guard rather than evidence for it.Deliberately not included
The issue also suggests consulting
FEATrather than attempting MLST and handling the failure every time. That is an optimisation rather than a correctness fix, and mixing the two would make this harder to review — happy to follow up separately.One note on running the tests: this branch is off
master, soTestMainstill requires./build_test_server.sh. I verified via the container harness in #73; the fix itself is independent of it.