Skip to content

Conversation

faucct
Copy link
Contributor

@faucct faucct commented Aug 3, 2023

A read-call with len == 0 happens here:
https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/master/src/java.base/share/classes/java/io/InputStream.java#L396

License

  • I confirm that this pull request can be released under the Apache 2 license

@debora-ito debora-ito added the needs-review This issue or PR needs review from the team. label Aug 15, 2023
@dagnir
Copy link
Contributor

dagnir commented Aug 23, 2023

Thanks @faucct. The change itself looks good; can you please add a test for this here?

@debora-ito debora-ito removed the needs-review This issue or PR needs review from the team. label Aug 24, 2023
@faucct
Copy link
Contributor Author

faucct commented Aug 29, 2023

I have tried adding the test for it, but don’t know, how to skip its compilation on Java 8. @dagnir , could you add it for me, please, if you know the way?

    @EnabledForJreRange(min = JRE.JAVA_9)
    @Test
    public void readAllBytesShouldReceiveNoPrematureEOS() throws IOException {
        publisher.subscribe(subscriber);
        publisher.send(byteBufferOfLength(2 << 14));
        publisher.complete();
        assertThat(subscriber.readAllBytes()).isEqualTo(new byte[2 << 14]);
    }

@dagnir
Copy link
Contributor

dagnir commented Sep 7, 2023

Hey sorry for the delayed response. No worries, I can add a test case.

Fixed some of the tests since the assertions fail if the read length is 0. This
is in line with the default implementation of `read(byte[],int,int)` in
`InputStream`:
https://github.com/openjdk/jdk11u/blob/d77215acdd6b9008d7f58c7ad5a82d6087c20f86/src/java.base/share/classes/java/io/InputStream.java#L267
Copy link
Contributor

@zoewangg zoewangg left a comment

Choose a reason for hiding this comment

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

Can we add a change log entry for this?

@dagnir dagnir enabled auto-merge (squash) September 12, 2023 20:28
@dagnir dagnir merged commit 3abf1c3 into aws:master Sep 12, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@dagnir
Copy link
Contributor

dagnir commented Sep 14, 2023

@all-contributors please add @faucct for code

@allcontributors
Copy link
Contributor

@dagnir

I've put up a pull request to add @faucct! 🎉

aws-sdk-java-automation added a commit that referenced this pull request Sep 3, 2025
…c8f9d0ea6

Pull request: release <- staging/0a34fbf5-9db6-403f-82cb-1bcc8f9d0ea6
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.

InputStreamSubscriber changes read() behaviour when passed len=0. Breaks InputStream.readAllBytes
5 participants