Skip to content

fix: respect read limit in BoundedInputStream.available()#779

Merged
garydgregory merged 9 commits intomasterfrom
feat/available-cleanup
Sep 8, 2025
Merged

fix: respect read limit in BoundedInputStream.available()#779
garydgregory merged 9 commits intomasterfrom
feat/available-cleanup

Conversation

@ppkarwasz
Copy link
Contributor

Note

This PR depends on the fixes proposed in #778. It should not be marked as ready for review until that PR has been approved or rejected.

BoundedInputStream.available() previously returned the underlying stream’s availability without applying the configured read limit. It now caps the reported value by the remaining bytes allowed by the bound.

Additional changes:

  • Removed the onMaxLength call from available(), since available() does not consume data and cannot exceed the maximum count.

  • Reworked the testAvailableAfterClose and testReadAfterClose tests:

    • Old versions relied too heavily on CharSequenceInputStream’s specific behavior.
    • New parameterized tests cover all three typical cases of the underlying stream after close: no-op, return special value, or throw an exception.
  • Added a dedicated unit test for the corrected available() behavior.

Before you push a pull request, review this list:

  • I used AI to improve unit tests and documentation.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best-practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

`BoundedInputStream.getRemaining()` previously returned `0` when no read limit was set, which was misleading.
It now returns `Long.MAX_VALUE` to more clearly represent an unbounded stream.

The Javadoc was also updated to clarify that the method reflects only the configured limit and does **not** report the actual number of bytes available in the underlying stream.
`BoundedInputStream.available()` previously returned the underlying stream’s availability without applying the configured read limit. It now caps the reported value by the remaining bytes allowed by the bound.

Additional changes:

* Removed the `onMaxLength` call from `available()`, since `available()` does not consume data and cannot exceed the maximum count.
* Reworked the `testAvailableAfterClose` and `testReadAfterClose` tests:

  * Old versions relied too heavily on `CharSequenceInputStream`’s specific behavior.
  * New parameterized tests cover all three typical cases of the underlying stream after close: no-op, return special value, or throw an exception.
* Added a dedicated unit test for the corrected `available()` behavior.
Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ppkarwasz

Good bug fix! 😄 I have comments scattered throughout.

@garydgregory garydgregory merged commit 86119d3 into master Sep 8, 2025
36 of 38 checks passed
@garydgregory
Copy link
Member

Merged, thank you @ppkarwasz.

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

Comments