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

GuavaCollectionDeserializer still throws NPE in some circumstances #138

Closed
arthurscchan opened this issue Dec 18, 2023 · 0 comments
Closed

Comments

@arthurscchan
Copy link
Contributor

In #124, the NullPointerException thrown from GuavaImmutableCollectionDeserializer::_deserializeContents(...) method is fixed by adding a _tryToAddNull() method to try if null could be added and wrap the NullPointerException if failed. But the call to the method is only added to the first branch of the conditional check. It is found that the second and third branches could also make value null. Thus the _tryToAddNull() method call should be moved outside of the conditional check, just before the builder.add(value) is called to ensure the value must not be null when invoking the builder.add(value) method.

        protected T _deserializeContents(JsonParser p, DeserializationContext ctxt)
        throws IOException
    {
        ...
        while ((t = p.nextToken()) != JsonToken.END_ARRAY) {
           ...
                if (value == null) {
                    if (value == null) {
                        _tryToAddNull(p, ctxt, builder);
                        continue;
                    }
                }
            } else if (typeDeser == null) {
                value = valueDes.deserialize(p, ctxt);
            } else {
                value = valueDes.deserializeWithType(p, ctxt, typeDeser);
            }

            builder.add(value);
        }
      ...

Fixes could be implemented by moving the _tryToAddNull() method call to the end of the conditional check, just before the invocation of the builder.add(value) method.

We found this issue by OSS-Fuzz and it is reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65117 after the fixes from #124 have been merged.

@cowtowncoder cowtowncoder changed the title GuavaCollectionDeserializer still throw NPE in some circumstances GuavaCollectionDeserializer still throws NPE in some circumstances Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants