Skip to content

Conversation

@wingo
Copy link
Contributor

@wingo wingo commented Dec 8, 2025

On Windows, it was possible to return a directory descriptor if READ wasn't in the permissions. Fixes wasmtime for
WebAssembly/wasi-testsuite#176.

@wingo wingo requested a review from a team as a code owner December 8, 2025 10:46
@wingo
Copy link
Contributor Author

wingo commented Dec 8, 2025

In all honesty I haven't tested on Windows, it's just that this patch should fix the issue. To repro, check out the prod/testsuite-base branch of https://github.com/WebAssembly/wasi-testsuite and run:

wasmtime -Wcomponent-model-async -Sp3,http --dir 'tests\rust\testsuite\wasm32-wasip3\fs-tests.dir::fs-tests.dir' 'tests\rust\testsuite\wasm32-wasip3\filesystem-flags-and-type.wasm'

@wingo wingo force-pushed the windows-open-dir-with-no-read branch from 2e830ac to 6cd0e89 Compare December 8, 2025 10:52
@wingo
Copy link
Contributor Author

wingo commented Dec 8, 2025

Rebased to fix the rustfmt error

@wingo wingo changed the title filesystem: ErrorCode::IsDirectory when opening director w/o READ filesystem: ErrorCode::IsDirectory when opening directory w/o READ Dec 8, 2025
On Windows, it was possible to return a directory descriptor if READ
wasn't in the permissions.  Fixes wasmtime for
WebAssembly/wasi-testsuite#176.
@wingo wingo force-pushed the windows-open-dir-with-no-read branch from 6cd0e89 to a7831bf Compare December 8, 2025 11:30
@rvolosatovs rvolosatovs enabled auto-merge December 8, 2025 11:38
@rvolosatovs rvolosatovs added this pull request to the merge queue Dec 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 8, 2025
@github-actions github-actions bot added the wasi Issues pertaining to WASI label Dec 8, 2025
@wingo
Copy link
Contributor Author

wingo commented Dec 8, 2025

My goodness, there are many CI checks in this project :) Patch fixes unused-variable warning/error when compiling for Windows.

@rvolosatovs rvolosatovs enabled auto-merge December 8, 2025 13:21
@rvolosatovs
Copy link
Member

My goodness, there are many CI checks in this project :) Patch fixes unused-variable warning/error when compiling for Windows.

You can add prtest:full to commit message description to trigger a full CI run for cases like these

@rvolosatovs rvolosatovs added this pull request to the merge queue Dec 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 8, 2025
@alexcrichton
Copy link
Member

Test-wise this is something that'll be caught when updating to the latest wasi-testsuite, right? If so, I think it's ok to skip a test here. Mind leaving a comment though on this match arm explaining that this is sync'ing windows semantics to unix semantics?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasi Issues pertaining to WASI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants