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.

Sign up for GitHub

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

Jump to bottom

{Boolean,Byte,Char,Double,Float,Int,Long,Short}s.concat() may throw {NegativeArraySize,ArrayIndexOutOfBounds}Exception #3303

Closed
ineuwirth opened this issue Oct 20, 2018 · 4 comments · Fixed by #7377
Closed

{Boolean,Byte,Char,Double,Float,Int,Long,Short}s.concat() may throw {NegativeArraySize,ArrayIndexOutOfBounds}Exception #3303

ineuwirth opened this issue Oct 20, 2018 · 4 comments · Fixed by #7377
Assignees
@nick-someone
Labels
P3 no SLO package=primitives status=triaged type=api-docs Change/add API documentation

Comments

@ineuwirth
Copy link
Contributor

ineuwirth commented Oct 20, 2018 •
edited
Loading

The concat() method in the per-type primitive utility classes throws unexpected NegativeArraySizeException or ArrayIndexOutOfBoundsException if the input arrays contain too many elements.

NegativeArraySizeException is being thrown if there is an overflow ending up with a negative int. In this case, the result array cannot be constructed with this number.

ArrayIndexOutOfBoundsException is being thrown if there is an overflow ending up with a non-negative int. In this case, the result array will be shorter than expected and System.arraycopy() will throw AIOOBE.
ineuwirth pushed a commit to ineuwirth/guava that referenced this issue Oct 20, 2018
The concat() method in the per-type primitive utility classes throw…
2572c99 
… unexpected `NegativeArraySizeException` or `ArrayIndexOutOfBoundsException` if the input arrays contain too many elements.

Added test cases to demonstrate both overflow cases. Possible fix is provided.

In this commit only the module `guava` and only the utility class `Longs` is covered along with its unit tests.

See google#3303
@ineuwirth ineuwirth mentioned this issue Oct 20, 2018
Closed
@ineuwirth
Copy link
Contributor Author

Test cases to demonstrate the issue can be found in #3304.

I am happy to cover other classes and modules (android, gwt) as well if you think the test cases/provided fix are good.

ineuwirth pushed a commit to ineuwirth/guava that referenced this issue Oct 21, 2018
The concat() method in the per-type primitive utility classes throw…
7a59adf 
… unexpected `NegativeArraySizeException` or `ArrayIndexOutOfBoundsException` if the input arrays contain too many elements.

De-duplicate unit test code.

See google#3303
@raghsriniv raghsriniv added the P3 no SLO label Jun 24, 2019
@kluever kluever removed the P3 no SLO label Oct 28, 2019
@kluever
Copy link
Member

kluever commented Oct 28, 2019

This isn't actually a bug, right? It's just improving the API docs?

@kluever kluever added type=api-docs Change/add API documentation P3 no SLO labels Oct 28, 2019
@ineuwirth
Copy link
Contributor Author

Well, a similar issue has been fixed in code instead of documenting the behavior e.g. in #3026.
Also, in case of an overflow ending up with a positive number, we could prevent a potentially big array from being allocated for nothing.

@copybara-service copybara-service bot mentioned this issue Jun 26, 2023
Closed
copybara-service bot pushed a commit that referenced this issue Jun 26, 2023
Changed Longs.concat() to throw IllegalArgumentException if the i…
ecce137 
…nput arrays contain too many elements.

See #3303

Fixes #3304

RELNOTES=n/a
PiperOrigin-RevId: 543414513
copybara-service bot pushed a commit that referenced this issue Jun 26, 2023
Changed Longs.concat() to throw IllegalArgumentException if the i…
8720b6b 
…nput arrays contain too many elements.

See #3303

Fixes #3304

RELNOTES=n/a
PiperOrigin-RevId: 543451555
@perceptron8 perceptron8 mentioned this issue Aug 27, 2024
Closed
copybara-service bot pushed a commit that referenced this issue Aug 27, 2024
Changed *.concat() to throw IllegalArgumentException if the input…
2538294 
… arrays contain too many elements.

Fixes #7376
Fixes #3303

This change comes from @perceptron8, who generalized the earlier PR by @ineuwirth.

RELNOTES=n/a
PiperOrigin-RevId: 667983976
@copybara-service copybara-service bot mentioned this issue Aug 27, 2024
Merged
copybara-service bot pushed a commit that referenced this issue Aug 27, 2024
Changed *.concat() to throw IllegalArgumentException if the input…
8bc8ea9 
… arrays contain too many elements.

Fixes #7376
Fixes #3303

This change comes from @perceptron8, who generalized the earlier PR by @ineuwirth.

RELNOTES=n/a
PiperOrigin-RevId: 667983976
@cpovirk
Copy link
Member

cpovirk commented Aug 27, 2024

I think the PR from last year and the followup today have covered all this, but let me know if I'm missing anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nick-someone nick-someone

Labels
P3 no SLO package=primitives status=triaged type=api-docs Change/add API documentation
Projects
None yet
Milestone
No milestone
6 participants
@nick-someone @ronshapiro @cpovirk @kluever @ineuwirth @raghsriniv