google-cloud-nio: CloudStorageReadChannel: fix critical bug that could cause the channel position to overflow by droazen · Pull Request #2283 · googleapis/google-cloud-java

droazen · 2017-07-27T17:58:48Z

This fixes a critical bug in CloudStorageReadChannel in which the channel position
(a long) was getting down-cast to int in CloudStorageReadChannel.innerOpen(). This
could cause the channel position to become negative or a wildly incorrect positive
value if the channel position was greater than MAX_INT.

In practice, this bug would only manifest itself if the channel was constructed with
a non-zero initial position, or if the channel was reopened in response to an error.

Included a test case that fails without the fix, and passes with the fix.

lbergelson · 2017-07-27T18:00:15Z

The long nightmare is almost over!

pshapiro4broad · 2017-07-27T18:19:24Z

👍

lbergelson · 2017-07-27T18:41:17Z

This will fix broadinstitute/gatk#2516 and broadinstitute/gatk#3316

coveralls · 2017-07-27T19:12:17Z

Changes Unknown when pulling 72215b5 on droazen:dr_fix_CloudStorageReadChannel_position_overflow into ** on GoogleCloudPlatform:master**.

…shaded This includes the fix for the position overflowing in CloudStorageReadChannel (googleapis/google-cloud-java#2283), as well as the fix for the intermittent 503 errors we've already been depending on (googleapis/google-cloud-java#2281)

…shaded (#3373) This includes the fix for the position overflowing in CloudStorageReadChannel (googleapis/google-cloud-java#2283), as well as the fix for the intermittent 503 errors we've already been depending on (googleapis/google-cloud-java#2281)

droazen · 2017-08-15T18:33:26Z

@garrettjonesgoogle Could we please get an official review on this PR and #2281? Without these patches google-cloud-nio is unusable at large scale, so as a large-scale user of this library we are naturally eager to see these make it into a future release! :)

This PR (#2283) fixes a nasty bug that could cause the channel position to overflow and become either negative or a wildly incorrect positive value, leading to corruption of the input stream from the client perspective. This is not a hypothetical problem -- we experienced it quite frequently!

The other PR (#2281) wraps additional GCS operations within retry loops. Without that patch, 10-20% of our jobs were failing with transient 503 errors.

Thanks for your help!

michaelbausor

LGTM, one nit from Codacy

Sign in to view

+ // This position value will overflow to a negative value if a long -> int cast is attempted
+ long startPosition = 11918483280L;
+ ArgumentCaptor<Long> captor = ArgumentCaptor.forClass(Long.class);
+ final CloudStorageReadChannel channel = CloudStorageReadChannel.create(gcsStorage, file, startPosition, 1);


…d cause the channel position to overflow This fixes a critical bug in CloudStorageReadChannel in which the channel position (a long) was getting down-cast to int in CloudStorageReadChannel.innerOpen(). This could cause the channel position to become negative or a wildly incorrect positive value if the channel position was greater than MAX_INT. In practice, this bug would only manifest itself if the channel was constructed with a non-zero initial position, or if the channel was reopened in response to an error. Included a test case that fails without the fix, and passes with the fix.

droazen · 2017-08-24T14:43:12Z

@michaelbausor Thanks for the review! I've addressed the codacy error, so this should be good to go now I think.

droazen · 2017-08-24T16:03:54Z

Thanks for merging this and #2281 @michaelbausor! Is there a timeline for the next google-cloud-java release?

michaelbausor · 2017-08-24T16:06:06Z

Re the timing of the next release, @garrettjonesgoogle will know best

…s#2283)

…low (#2283) This fixes a critical bug in CloudStorageReadChannel in which the channel position (a long) was getting down-cast to int in CloudStorageReadChannel.innerOpen(). This could cause the channel position to become negative or a wildly incorrect positive value if the channel position was greater than MAX_INT. In practice, this bug would only manifest itself if the channel was constructed with a non-zero initial position, or if the channel was reopened in response to an error. Included a test case that fails without the fix, and passes with the fix.

* chore(main): release 3.35.0 * chore: generate libraries at Thu Jan 15 15:40:14 UTC 2026 --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>

…eapis#2283) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.truth:truth](https://togithub.com/google/truth) | `1.4.3` -> `1.4.4` | [![age](https://developer.mend.io/api/mc/badges/age/maven/com.google.truth:truth/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.google.truth:truth/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.google.truth:truth/1.4.3/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.google.truth:truth/1.4.3/1.4.4?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>google/truth (com.google.truth:truth)</summary> ### [`v1.4.4`](https://togithub.com/google/truth/releases/tag/v1.4.4) [Compare Source](https://togithub.com/google/truth/compare/v1.4.3...v1.4.4) - Annotated the rest of the main package for nullness, and moved the `@NullMarked` annotation from individual classes up to the package to avoid [a warning under `--release 8`](https://togithub.com/google/truth/issues/1320). ([`e107aea`](https://togithub.com/google/truth/commit/e107aeadc)) - Improved the failure message for `matches` to conditionally suggest using `containsMatch`. ([`7e9fc7a`](https://togithub.com/google/truth/commit/7e9fc7aec)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/googleapis/java-bigtable).

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 27, 2017

droazen mentioned this pull request Jul 27, 2017
Error in BaseRecalibrator while using NIO broadinstitute/gatk#3316
Closed

droazen mentioned this pull request Jul 27, 2017
Update google-cloud-nio dependency to 0.20.4-alpha-20170727.190814-1:shaded broadinstitute/gatk#3373
Merged

droazen mentioned this pull request Aug 1, 2017
NIO error: "position should be non-negative" broadinstitute/gatk#2516
Closed

michaelbausor reviewed Aug 21, 2017
View reviewed changes

droazen force-pushed the dr_fix_CloudStorageReadChannel_position_overflow branch from 72215b5 to 398cdaaCompare August 24, 2017 14:40

michaelbausor approved these changes Aug 24, 2017
View reviewed changes

michaelbausor merged commit 4055f1f into googleapis:masterAug 24, 2017

droazen mentioned this pull request Aug 24, 2017
Update to next google-cloud-java release once it's out broadinstitute/gatk#3500
Closed

suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Mar 23, 2026
deps: update dependency commons-cli:commons-cli to v1.11.0 (googleapi…
7e716fc
…s#2283)

Navigation Menu

Uh oh!

google-cloud-nio: CloudStorageReadChannel: fix critical bug that could cause the channel position to overflow - #2283

google-cloud-nio: CloudStorageReadChannel: fix critical bug that could cause the channel position to overflow#2283
michaelbausor merged 1 commit into
googleapis:masterfrom
droazen:dr_fix_CloudStorageReadChannel_position_overflow

droazen commented Jul 27, 2017

Uh oh!

lbergelson commented Jul 27, 2017

Uh oh!

pshapiro4broad commented Jul 27, 2017

Uh oh!

lbergelson commented Jul 27, 2017

Uh oh!

coveralls commented Jul 27, 2017

Uh oh!

droazen commented Aug 15, 2017

Uh oh!

michaelbausor left a comment

Uh oh!

This comment was marked as spam.

Uh oh!

droazen commented Aug 24, 2017

Uh oh!

droazen commented Aug 24, 2017

Uh oh!

michaelbausor commented Aug 24, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

droazen commented Jul 27, 2017

Uh oh!

lbergelson commented Jul 27, 2017

Uh oh!

pshapiro4broad commented Jul 27, 2017

Uh oh!

lbergelson commented Jul 27, 2017

Uh oh!

coveralls commented Jul 27, 2017

Uh oh!

droazen commented Aug 15, 2017

Uh oh!

michaelbausor left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

droazen commented Aug 24, 2017

Uh oh!

droazen commented Aug 24, 2017

Uh oh!

michaelbausor commented Aug 24, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants