One Range parser, and a path that leaves the volume is a refusal - #441
Merged
Merged
Conversation
`routes/files/preview.js` and `routes/shares.js` each carried their own copy of the byte-range parsing. Two copies of anything drift, and these had: the preview route sets security headers on a partial response that the share route does not, so the same file came back with different headers depending on which address it was asked for. `utils/httpRange.js` does it once. Both routes call it, and neither parses a Range header any more. Found while porting the preview route, and fixed here because it is that route's own test that shows it: `normalizeRelativePath` threw a plain `Error` for a path that leaves the volume, and a plain Error reaches the browser as a 500. So `../../../etc/passwd` answered "500 Internal Server Error" — a server fault, and a status a caller retries — for something that is the request's fault and will never succeed. It is a `ValidationError` now, which is a 400. ## Checks `http-range.test.js`, 10 tests over the parser itself: a header that is not `bytes=`, a start past the end, an open-ended range, a suffix range, a size of zero. Making a malformed header parse as an absent one turns one of them red. `preview.test.js`, 20 tests, including the two path-traversal ones that were the 500. Whole backend suite: 2 476 passed, 2 failed — the two that fail on `main` on its own. `download.js` was in this batch at first and is not any more: its zip building uses a newer `archiver` than `main` has, which is a dependency question and belongs to the batch that answers it. It parses no Range header, so nothing of this subject is left behind there.
cerede2000
pushed a commit
to cerede2000/NextExplorer
that referenced
this pull request
Sep 26, 2026
The batch is reversed, so its findings move from PORT to DONE in the same commit that sends them. What is left to reverse is whatever `scripts/parity.mjs` still reports.
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.
P3-09 of phase 3 (see #373). Stacked on #433–#440.
Two copies of the same parsing, and they disagree
routes/files/preview.jsandroutes/shares.jseach parse theRangeheaderthemselves — the same
bytes=prefix, the same clamping, the same 416s, written twice:And they have drifted, which is what two copies of anything do: the preview route puts
security headers on a partial response that the share route does not, so the same file
comes back with different headers depending on which address it was asked for.
utils/httpRange.jsdoes it once. Both routes call it and neither parses a Range headerany more.
A path that leaves the volume answered 500
Found while porting the preview route, and fixed here because it is that route's own test
that shows it:
A plain
Errorreaches the browser as a 500. So../../../etc/passwdanswered 500Internal Server Error — a server fault, and a status a caller retries — for something
that is the request's fault and will never succeed. It is a
ValidationErrornow, whichis a 400.
Checks
http-range.test.js, 10 tests over the parser itself: a header that is notbytes=, a start past the end, an open-ended range, a suffix range, a size of zero.Making a malformed header parse as an absent one turns one red.
preview.test.js, 20 tests, including the two path-traversal cases that were the500.
mainalone.npm run lintup by the two parse errors the new test files draw,the same one 141 of
main's own test files already draw.download.jswas in this batch at first and is not any more: its zip building wants anewer
archiverthanmainhas, which is a dependency question for the batch thatanswers it. It parses no Range header, so nothing of this subject is left behind there.