Skip to content

fix(files): bound SeekableHttpStream reads by Content-Range - #64086

Draft
joshtrichards wants to merge 5 commits into
masterfrom
jtr/fix-seekableHTTP-bounding
Draft

fix(files): bound SeekableHttpStream reads by Content-Range#64086
joshtrichards wants to merge 5 commits into
masterfrom
jtr/fix-seekableHTTP-bounding

Conversation

@joshtrichards

Copy link
Copy Markdown
Member

Summary

Harden SeekableHttpStream range handling by making the resource size declared in Content-Range authoritative.

The key read-boundary change is:

$remaining = $this->totalSize - $this->offset;
$ret = fread($stream, min($count, $remaining));

Reads are now limited to the remaining bytes in the remote resource, so the wrapper no longer requests data beyond the logical end simply because an HTTP connection remains open.

The second commit hardens Content-Range validation through a dedicated helper and makes stream lifecycle and reconnect failure handling more explicit. It also ensures that the total resource size remains consistent across range requests.

Logical EOF is determined from the declared resource size rather than transport connection closure. No additional feof() check is introduced, since probing a socket for transport-level EOF could add blocking behavior. A response shorter than its declared Content-Range remains governed by the existing PHP stream semantics.

This addresses the timeout reported in #58276 while also improving general range-stream correctness and adding focused explicit test coverage.

TODO

  • Backport?

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Limit reads to the remaining bytes reported by `Content-Range` and determine EOF from the logical resource offset instead of the underlying HTTP stream. This avoids reading beyond the remote resource boundary when the response connection remains open.

The change is safe because this wrapper already requires `Content-Range` and uses its total-size component to establish the remote resource boundary; the read cap is consistent with the existing contract.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Replace the ambiguous range-length state with an explicit total size, validate Content-Range responses, and improve stream cleanup during
reconnect failures.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Assisted-by: Copilot:gpt-5.6-luna

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: SeekableHttpStream Relies on Connection Close Instead of Content-Length, Causing 60s Timeout

2 participants