Skip to content

fix(files): correct quota stream accounting - #64087

Draft
joshtrichards wants to merge 6 commits into
masterfrom
jtr/fix-files-stream-accounting-reading
Draft

fix(files): correct quota stream accounting#64087
joshtrichards wants to merge 6 commits into
masterfrom
jtr/fix-files-stream-accounting-reading

Conversation

@joshtrichards

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

Correct stream quota accounting and add regression coverage for short reads, short writes, and failed seeks.

Problem:

The quota stream tracks a mutable allowance for each wrapped stream. Several edge cases could make that allowance diverge from the actual stream state:

  • stream_read() reduced the allowance by the requested byte count, even when the underlying stream returned fewer bytes at EOF. This could consume allowance for bytes that were not actually read.
  • stream_seek() adjusted the allowance before knowing whether the underlying seek succeeded. A failed seek could therefore change quota accounting.
  • Seeking beyond the quota could make the allowance negative. That value was then passed as the length to substr(), where PHP interprets it as a negative length rather than as zero available bytes.
  • Before fix(files): decrement quota by actual bytes written in stream_write #55731, stream_write() reduced the allowance by the intended write size instead of the number of bytes actually written.

Changes:

  • Decrement the allowance by the number of bytes actually read.
  • Decrement the allowance by the number of bytes actually written.
  • Update the allowance only after a seek succeeds.
  • Restore the original stream position on failed seeks on a best-effort basis, without changing the allowance.
  • Explicitly reject writes when the remaining allowance is non-positive, preventing negative allowances from being interpreted as negative substr() lengths.
  • Document the allowance as an int|float byte count.
  • Add regression tests for:
    • short writes from the underlying stream;
    • short reads at EOF;
    • failed SEEK_SET and SEEK_END operations;
    • writes after seeking beyond the quota.

The short-write regression test also covers the behavior fixed by #55731.

TODO

  • Backport?

Checklist

AI (if applicable)

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

Follow-up to PR #55731

Account for bytes actually read, preserve quota state on failed seeks, and prevent writes when the remaining allowance is exhausted. Document
the per-stream allowance and support float quota values.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Add regression coverage for short reads, failed seeks, exhausted allowances, and short writes where the underlying stream writes fewer bytes than requested. Includes coverage for related PR #55731.

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>
Add coverage for failed end-relative seeks, negative remaining allowances, and float quota limits.

Assisted-by: Copilot:gpt-5.6-luna

Signed-off-by: Josh <josh.t.richards@gmail.com>
Not realistic to test and not really needed anyhow...

Signed-off-by: Josh <josh.t.richards@gmail.com>
Detect the test payload and return zero for retry data so the test consistently verifies accounting for bytes actually written.

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.

2 participants