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

JsonLocation in 2.13 only uses identity comparison for "content reference" #739

Closed
vladt opened this issue Jan 10, 2022 · 4 comments
Closed
Milestone

Comments

@vladt
Copy link

vladt commented Jan 10, 2022

In 2.12, JsonLocation.equals() uses equals() to check equality on JsonLocation._sourceRef
https://github.com/FasterXML/jackson-core/blob/2.12/src/main/java/com/fasterxml/jackson/core/JsonLocation.java#L150

In 2.13, JsonLocation._sourceRef was renamed and moved to ContentReference.
JsonLocation.equals() still uses equals() to check equality on JsonLocation._contentReference:
https://github.com/FasterXML/jackson-core/blob/2.13/src/main/java/com/fasterxml/jackson/core/JsonLocation.java#L276
but ContentReference.equals() uses == to check equality on ContentReference._rawContent:
https://github.com/FasterXML/jackson-core/blob/2.13/src/main/java/com/fasterxml/jackson/core/io/ContentReference.java#L360

This changes the behaviour of JsonLocation.equals() between 2.12 and 2.13, breaking client code that relies on JsonLocation.equals().

@cowtowncoder
Copy link
Member

I am not sure there is much that can be done here, unfortunately, since it is uncertain if calling equals() on arbitrary source reference is safe or not.
(one could argue that not checking _offset or _length is wrong tho).

But I would be interested in knowing specific use case: what is the content reference in specific case you encounter?
Maybe that case could be addressed.

@vladt
Copy link
Author

vladt commented Jan 12, 2022

In my case, the source/content reference is a java.io.File.

Here's a small class to reproduce the problem. It prints "true" for 2.12.5 and "false" for 2.13.1.

package mytest;

import java.io.File;

import com.fasterxml.jackson.core.JsonLocation;

public class TestJsonLocation
{
  public static void main(String[] args) {
    JsonLocation jsonLocation1 = new JsonLocation(new File("foo.json"), 1, 2, 3);
    JsonLocation jsonLocation2 = new JsonLocation(new File("foo.json"), 1, 2, 3);
    System.out.println(jsonLocation1.equals(jsonLocation2));
  }
}

@cowtowncoder
Copy link
Member

Yeah this is tricky. I guess it'd be possible to implement logic for some well-known types like:

  • java.io.File
  • java.net.URL
  • maybe String?

to consider equality. It'd be clunky and not extensible. But would resolve the immediate issue.

@cowtowncoder cowtowncoder added this to the 2.13.2 milestone Jan 17, 2022
@cowtowncoder cowtowncoder changed the title Breaking change in JsonLocation.equals() between 2.12 and 2.13 JsonLocation in 2.13 only uses identity comparison for "content reference" Jan 17, 2022
@vladt
Copy link
Author

vladt commented Jan 18, 2022

Please see my comment at c5aa1e1#r64115421
I'm not sure if it's valid though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants