-
Notifications
You must be signed in to change notification settings - Fork 3.4k
FetchFS: getFileRange
issues out-of-bounds chunk requests
#24438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
It looks good to me! |
This change is authored by @foxtacles (See emscripten-core#24438) Fixes: emscripten-core#24438
I opened #24442 with this patch. But we really should write a test too. Do either of you know how to re-produce the issue and make a test? |
I know how to re-produce it in principle (simply by trying to read from the file past the end of it). However I can't get the local tests to work (yet), they are all failing, so I can't write a test right now:
Probably missing something obvious since it's my first time running the tests. Other tests seem to pass. |
I've managed to get the tests running using Firefox. Unfortunately, reproducing this issue with tests turned out much harder for 2 reasons:
|
If you are referring to test server using by the emscripten test suite that seems like something we should be able to fix (re generating 416 error). The fact that you cannot make it happen using normal read calls is more worrying. If you are able to reproduce in your full app can you capture a full backtrace so we can see where call comes from. |
Uh oh!
There was an error while loading. Please reload this page.
Version of emscripten/emsdk:
The
getFileRange
function in FetchFS currently allows requests for chunks outside the file boundaries. The server will then respond with416 Range Not Satisfiable
, which is properly handled in the code but I think we should save ourselves the request in the first place.I've applied the attached patch to resolve this problem. I'd submit a pull request but I don't have the time right now to set up a local test environment.
CC @JoeOsborn
bounds.patch
The text was updated successfully, but these errors were encountered: