Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 1303 solved. #1330

Merged
merged 4 commits into from
Oct 2, 2024
Merged

Issue 1303 solved. #1330

merged 4 commits into from
Oct 2, 2024

Conversation

UFA-MOROZOV
Copy link
Contributor

@UFA-MOROZOV UFA-MOROZOV commented Sep 23, 2024

@yegor256, please review

@UFA-MOROZOV UFA-MOROZOV reopened this Sep 27, 2024
@yegor256
Copy link
Owner

@UFA-MOROZOV how do know it's solved? Maybe you can create a unit test that reproduces the problem first?

@UFA-MOROZOV
Copy link
Contributor Author

UFA-MOROZOV commented Sep 30, 2024

@UFA-MOROZOV how do know it's solved? Maybe you can create a unit test that reproduces the problem first?

@yegor256, i used this test that i added in ChunkedInputStreamTest locally(Before my changes in code it didn't work as was stated in issue):

    @Test
    void readsWithLenGreaterThanTotalSize() throws IOException {
        final String data = "Hello, World!";
        final String length = Integer.toHexString(data.length());
        final InputStream stream = new ChunkedInputStream(
                IOUtils.toInputStream(
                        new Joined(
                                ChunkedInputStreamTest.CRLF,
                                length,
                                data,
                                ChunkedInputStreamTest.END_OF_CHUNK,
                                ""
                        ).toString(),
                        StandardCharsets.UTF_8
                )
        );
        final byte[] buf = new byte[data.length() + 10];
        final int bytesRead = stream.read(buf);
        MatcherAssert.assertThat(bytesRead, Matchers.equalTo(data.length()));
        MatcherAssert.assertThat(buf, Matchers.equalTo((data + new String(new byte[10])).getBytes()));
        MatcherAssert.assertThat(stream.available(), Matchers.equalTo(0));
        stream.close();
    }

@UFA-MOROZOV
Copy link
Contributor Author

@yegor256, i can add this test to corresponding file in repository, but i don't think it's necessary, since it not so common issue in most cases.

@yegor256
Copy link
Owner

yegor256 commented Oct 1, 2024

@UFA-MOROZOV we usually don't make changes without tests that confirm that the changes are needed

@UFA-MOROZOV
Copy link
Contributor Author

error
@yegor256, i commited updated ChunkedInputStreamTest and run it with and without changes in ChunkedInputStream. Result without changes creates an error, where "Hello, world!" is considered as 12 symbols because of said Issue. I attached screenshots of test without changing ChunkedInputStream.

@yegor256
Copy link
Owner

yegor256 commented Oct 2, 2024

@UFA-MOROZOV now it looks very good. However, the formatting of the code is a bit off. See what qulice is saying. Run the build locally: mvn clean install -Pqulice

@UFA-MOROZOV
Copy link
Contributor Author

@yegor256 i refactored code and ran qulice, now it is alright.

@yegor256
Copy link
Owner

yegor256 commented Oct 2, 2024

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Oct 2, 2024

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 3657add into yegor256:master Oct 2, 2024
13 checks passed
@rultor
Copy link
Collaborator

rultor commented Oct 2, 2024

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 26min)

@yegor256
Copy link
Owner

yegor256 commented Oct 2, 2024

@UFA-MOROZOV thanks!

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.

3 participants