Skip to content

ParameterBindingJsonReader fails with NPE when a bindable value for ObjectId parameter is null #4282

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

Closed
Matin69 opened this issue Feb 1, 2023 · 4 comments
Assignees
Labels
type: bug A general bug

Comments

@Matin69
Copy link

Matin69 commented Feb 1, 2023

In JSON queries when you have a nullable value for an object id parameter, A null pointer exception will be thrown. It may seem reasonable but it's not suitable for a case where you want to implement a search function using nullable parameter values. If the value is null you expect null to be replaced with object id.
My case :

    @Query("{ $and : [ { $or : [ { $expr: { $eq: ['?0', 'null'] } } , { movie_id : ObjectId(?0) } ] }, { $or : [ { $expr: { $eq: ['?1', 'null'] } } , { email : ?1 } ] } ] }")
    List<Comment> search(String movieId, String email, Pageable pageable);

The exception :

java.lang.NullPointerException: Cannot invoke "Object.toString()" because the return value of "org.springframework.data.mongodb.util.json.ParameterBindingJsonReader$BindableValue.getValue()" is null
	at org.springframework.data.mongodb.util.json.ParameterBindingJsonReader.readStringFromExtendedJson(ParameterBindingJsonReader.java:1428) ~[spring-data-mongodb-4.0.0.jar:4.0.0]

At least a better exception with a good message should be thrown because it takes some time for me to find out the exception reason

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 1, 2023
@christophstrobl christophstrobl self-assigned this Feb 1, 2023
@christophstrobl christophstrobl added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 1, 2023
@christophstrobl
Copy link
Member

sounds like a bug - thanks for reporting - we'll look into it!

@Matin69
Copy link
Author

Matin69 commented Feb 1, 2023

sounds like a bug - thanks for reporting - we'll look into it!

I opened a pull request that throws an exception with a good message in cases when a bindable value for string parameters is null

@christophstrobl
Copy link
Member

thanks @Matin69!

@christophstrobl
Copy link
Member

After having a 2nd look at the issue I do think that ParameterBindingJsonReader#readStringFromExtendedJson should be changed to avoid the NPE and return null if the bindable parameter value is actually null.
However, ObjectId(?0) should throw an IllegalArgumentException in case the parameter value is null aligning the behaviour to new ObjectId((String) null).

christophstrobl added a commit that referenced this issue Mar 21, 2023
…xtendedJson

This commit makes sure to return null for a null parameter value avoiding a potential NPE when parsing data.
In doing so we can ensure object creation is done with the intended value that may or may not lead to a downstream error eg. when trying to create an ObjectId with a null hexString.

Closes: #4282
@mp911de mp911de added this to the 4.0.5 (2022.0.5) milestone Apr 13, 2023
@mp911de mp911de changed the title ParameterBindingJsonReader Throw null pointer exception when a bindable value for a ObjectId parameter is null ParameterBindingJsonReader fails with NPE when a bindable value for ObjectId parameter is null Apr 13, 2023
mp911de pushed a commit that referenced this issue Apr 13, 2023
…ExtendedJson`.

This commit makes sure to return null for a null parameter value avoiding a potential NPE when parsing data.
In doing so we can ensure object creation is done with the intended value that may or may not lead to a downstream error eg. when trying to create an ObjectId with a null hexString.

Closes: #4282
Original pull request: #4334
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants