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

Fix throwIfFalseEOF if skip method was called before read #447

Merged
merged 5 commits into from
Aug 29, 2018
Merged

Fix throwIfFalseEOF if skip method was called before read #447

merged 5 commits into from
Aug 29, 2018

Conversation

medb
Copy link
Contributor

@medb medb commented Aug 16, 2018

throwIfFalseEOF method don't take into account bytes read/skipped in skip method and throws IOException when legit EOF was reached.

@medb medb changed the title Fix throwIfFalseEOF when using skip method Fix throwIfFalseEOF with skip method Aug 16, 2018
@medb medb changed the title Fix throwIfFalseEOF with skip method Fix throwIfFalseEOF if skip method was called before read Aug 16, 2018
@chingor13 chingor13 self-assigned this Aug 29, 2018
@chingor13 chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 29, 2018
@chingor13
Copy link
Collaborator

Can we add a test for this?

It looks like we can add something like this:

  public void testSkippingBytes() throws IOException {
    MockHttpURLConnection connection = new MockHttpURLConnection(null)
        .setResponseCode(200)
        .setInputStream(new ByteArrayInputStream(StringUtils.getBytesUtf8("0123456789")))
        .addHeader("Content-Length", "10");
    NetHttpResponse response = new NetHttpResponse(connection);
    InputStream is = response.getContent();
    // read 1 byte, then skip 9 (to EOF)
    assertEquals('0', is.read());
    assertEquals(9, is.skip(9));
    // expect EOF, not an exception
    assertEquals(-1, is.read());
  }

@medb medb closed this Aug 29, 2018
@medb medb deleted the patch-1 branch August 29, 2018 22:29
@medb medb restored the patch-1 branch August 29, 2018 22:29
@medb
Copy link
Contributor Author

medb commented Aug 29, 2018

Thanks for the test, added it to PR

@medb medb reopened this Aug 29, 2018
@chingor13 chingor13 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 29, 2018
@chingor13
Copy link
Collaborator

@medb Is it possible to rebase these changes off of the "dev" branch? Strangely, the "dev" branch is the primary branch, not "master" and the CI config in that branch cannot be run.

throwIfFalseEOF method don't take into account bytes read/skipped in skip method and throws IOException when legit EOF was reached.
@medb medb changed the base branch from master to dev August 29, 2018 23:15
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@medb
Copy link
Contributor Author

medb commented Aug 29, 2018

Rebased on dev branch

@googlebot
Copy link

CLAs look good, thanks!

@chingor13 chingor13 merged commit c194b3b into googleapis:dev Aug 29, 2018
@chingor13
Copy link
Collaborator

Thanks!

This was referenced Oct 12, 2018
@sduskis sduskis removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2019
clundin25 pushed a commit to clundin25/google-http-java-client that referenced this pull request Aug 11, 2022
This commit is to trigger automation for a new release.
See googleapis#446 for actual change.
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.

None yet

4 participants