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 missing bounds check for JsonTreeReader.getPath() #2001

Conversation

Marcono1234
Copy link
Collaborator

@Marcono1234 Marcono1234 commented Oct 25, 2021

There are situations where the stack of JsonTreeReader contains a JsonArray or JsonObject without a subsequent Iterator, for example after calling peek() or nextName().
When JsonTreeReader.getPath() is called afterwards it therefore must not assume that a JsonArray or JsonObject is always followed by an Iterator.

The only reason why this never caused an ArrayIndexOutOfBoundsException in the past is because the stack has an even default size (32) so it would just have read the next null.
However, if the stack had for example the default size 31, a user created a JsonTreeReader for 16 JSON arrays nested inside each other, then called 15 times beginArray(), followed by peek() and getPath() the exception would occur.

@google-cla google-cla bot added the cla: yes label Oct 25, 2021
There are situations where the stack of JsonTreeReader contains a JsonArray
or JsonObject without a subsequent Iterator, for example after calling peek()
or nextName().
When JsonTreeReader.getPath() is called afterwards it therefore must not
assume that a JsonArray or JsonObject is always followed by an Iterator.

The only reason why this never caused an ArrayIndexOutOfBoundsException in
the past is because the stack has an even default size (32) so it would just
have read the next `null`.
However, if the stack had for example the default size 31, a user created a
JsonTreeReader for 16 JSON arrays nested inside each other, then called 15
times beginArray(), followed by peek() and getPath() the exception would
occur.
@Marcono1234 Marcono1234 force-pushed the marcono1234/JsonTreeReader-getPath-bounds-check branch from ab0bb47 to 525e14a Compare October 25, 2021 01:38
@eamonnmcmanus
Copy link
Member

Thanks, good find!

@eamonnmcmanus eamonnmcmanus merged commit ba96d53 into google:master Oct 25, 2021
@Marcono1234 Marcono1234 deleted the marcono1234/JsonTreeReader-getPath-bounds-check branch October 25, 2021 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants