Skip to content

One Range parser, and a path that leaves the volume is a refusal - #441

Merged
vikramsoni2 merged 1 commit into
nxzai:mainfrom
cerede2000:p3-09
Sep 26, 2026
Merged

vikramsoni2 merged 1 commit into
nxzai:mainfrom
cerede2000:p3-09

Conversation

@cerede2000

Copy link
Copy Markdown

P3-09 of phase 3 (see #373). Stacked on #433–#440.

Two copies of the same parsing, and they disagree

routes/files/preview.js and routes/shares.js each parse the Range header
themselves — the same bytes= prefix, the same clamping, the same 416s, written twice:

$ git grep -c bytesPrefix -- backend/src
backend/src/routes/files/preview.js:1
backend/src/routes/shares.js:4

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.js does it once. Both routes call it and neither parses a Range header
any 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:

throw new Error('Invalid path. Traversal outside the volume root is not allowed.');

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 red.
  • preview.test.js, 20 tests, including the two path-traversal cases that were the
    500.
  • Whole backend suite: 2 476 passed, 2 failed — the two that fail on main alone.
  • Formatting unchanged; npm run lint up by the two parse errors the new test files draw,
    the same one 141 of main's own test files already draw.

download.js was in this batch at first and is not any more: its zip building wants a
newer archiver than main has, which is a dependency question for the batch that
answers it. It parses no Range header, so nothing of this subject is left behind there.

`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.
@vikramsoni2
vikramsoni2 merged commit b49bf79 into nxzai:main Sep 26, 2026
1 check passed
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