Skip to content

Treat STATUS_NO_SUCH_FILE as end of enumeration#358

Merged
jborean93 merged 1 commit into
jborean93:masterfrom
henrik-nil-acc:pr/scandir-empty-dir
Jun 25, 2026
Merged

Treat STATUS_NO_SUCH_FILE as end of enumeration#358
jborean93 merged 1 commit into
jborean93:masterfrom
henrik-nil-acc:pr/scandir-empty-dir

Conversation

@henrik-nil-acc

@henrik-nil-acc henrik-nil-acc commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

MS-SMB2 3.3.5.18 permits servers to terminate QUERY_DIRECTORY
with either STATUS_NO_MORE_FILES or STATUS_NO_SUCH_FILE. Samba
returns the latter on a non-matching pattern, but existing code
caught only the first. The escaping NoSuchFile is not an
OSError subclass, so it slips past os-style except clauses and
rmtree's ignore_errors.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.73%. Comparing base (3b5b3e9) to head (e68d4f5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
- Coverage   98.78%   98.73%   -0.06%     
==========================================
  Files          24       24              
  Lines        5288     5288              
==========================================
- Hits         5224     5221       -3     
- Misses         64       67       +3     
Flag Coverage Δ
macOS 69.09% <0.00%> (ø)
py3.10 98.73% <100.00%> (-0.06%) ⬇️
py3.11 98.73% <100.00%> (-0.06%) ⬇️
py3.12 98.73% <100.00%> (-0.06%) ⬇️
py3.13 98.73% <100.00%> (-0.06%) ⬇️
py3.14 98.73% <100.00%> (-0.06%) ⬇️
ubuntu 96.65% <100.00%> (-0.06%) ⬇️
windows 98.65% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jborean93

Copy link
Copy Markdown
Owner

Does this actually happen in real life with an SMB implementation I don't know of? I tried it against a Windows host and it didn't report this error just no items to yield. The fact that the test has to have a mock to represent this type of error also tells me that maybe Samba doesn't do this. While the comment indicate that MS-SMB2 shows STATUS_NO_SUCH_FILE can be returned it doesn't say why and if I can't see it through Windows then I'm wondering if it is returned for another reason that we don't want to ignore.

@henrik-nil-acc

Copy link
Copy Markdown
Contributor Author

Does this actually happen in real life with an SMB implementation I don't know of? I tried it against a Windows host and it didn't report this error just no items to yield. The fact that the test has to have a mock to represent this type of error also tells me that maybe Samba doesn't do this. While the comment indicate that MS-SMB2 shows STATUS_NO_SUCH_FILE can be returned it doesn't say why and if I can't see it through Windows then I'm wondering if it is returned for another reason that we don't want to ignore.

It happens with samba. New integration tests should be failing before the change. Also enabling debug logs should make it visible.

@henrik-nil-acc henrik-nil-acc changed the title Fix scandir error on empty directories Treat STATUS_NO_SUCH_FILE as end of enumeration Jun 24, 2026
@jborean93

jborean93 commented Jun 25, 2026

Copy link
Copy Markdown
Owner

It happens with samba

Ah I see, it was the non-matching pattern that did the trick. I was just doing scandir on an empty directory with the default pattern *. Looks like MS' SMB server does the same thing with a pattern that doesn't match.

Would it be possible to adjust the comments to state this occurs when the pattern doesn't match any files. You can keep the reference saying it may also occur on other servers normally but I just want to be clear when it normally happens in the code.

MS-SMB2 3.3.5.18 permits servers to terminate QUERY_DIRECTORY
with either STATUS_NO_MORE_FILES or STATUS_NO_SUCH_FILE. Samba
returns the latter on a non-matching pattern, but existing code
caught only the first. The escaping NoSuchFile is not an
OSError subclass, so it slips past os-style except clauses and
rmtree's ignore_errors.
@henrik-nil-acc

Copy link
Copy Markdown
Contributor Author

Better now?
Thanks for the reviews, I have a few more similar fixes, will open PRs in a slow pace :)

@jborean93
jborean93 merged commit b23a216 into jborean93:master Jun 25, 2026
19 checks passed
@jborean93

Copy link
Copy Markdown
Owner

Yea looks great.

Thanks for the reviews, I have a few more similar fixes, will open PRs in a slow pace :)

Thank you very much for testing these things out and sending the PRs. I greatly appreciate them being in small chunks as it helps to review them when I find some spare time. Let me know when you think you are nearing the end and I'll prep a new release.

@henrik-nil-acc
henrik-nil-acc deleted the pr/scandir-empty-dir branch June 26, 2026 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants