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

FromStringDeserializer is trying to convert a FIELD_NAME token instead of a VALUE_STRING #4651

Closed
1 task done
libetl opened this issue Jul 28, 2024 · 17 comments
Closed
1 task done
Labels
to-evaluate Issue that has been received but not yet evaluated

Comments

@libetl
Copy link
Contributor

libetl commented Jul 28, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

I'm using a JsonTypeInfo to tell the parser to look for sub types. There is a $$element property that matches the class simple name, so I'm using this type info mapping:

@JsonTypeInfo.Value(JsonTypeInfo.Id.SIMPLE_NAME, JsonTypeInfo.As.PROPERTY,
                    "$$element", null, false, false)

While I could see the practice working fine over a few examples, there was one case that resulted in systematic error :

Here is my json that I need to deserialize :

{"url":"https://localhost","$$element":"HttpURI","scheme":"https","authority":"localhost","path":"","relativePath":"","value":"https://localhost"}

url is of type java.net.URL.

The execution runs the method AsPropertyTypeDeserializer._deserializeTypedForId (from deserializeTypedFromObject, as soon as the type id property is scanned)

protected Object _deserializeTypedForId(JsonParser p, DeserializationContext ctxt,
TokenBuffer tb, String typeId) throws IOException
{
JsonDeserializer<Object> deser = _findDeserializer(ctxt, typeId);
if (_typeIdVisible) { // need to merge id back in JSON input?
if (tb == null) {
tb = ctxt.bufferForInputBuffering(p);
}
tb.writeFieldName(p.currentName());
tb.writeString(typeId);
}
if (tb != null) { // need to put back skipped properties?
// 02-Jul-2016, tatu: Depending on for JsonParserSequence is initialized it may
// try to access current token; ensure there isn't one
p.clearCurrentToken();
p = JsonParserSequence.createFlattened(false, tb.asParser(p), p);
}
if (p.currentToken() != JsonToken.END_OBJECT) {
// Must point to the next value; tb had no current, p pointed to VALUE_STRING:
p.nextToken(); // to skip past String value
}
// deserializer should take care of closing END_OBJECT as well
return deser.deserialize(p, ctxt);
}

a new JsonParser is created with the concatenation of the fields read prior to the type id identification and the remaining fields to be converted (i'm skipping the id $$element since I'm not longer needing it). This is the result of the statement JsonParserSequence.createFlattened(false, tb.asParser(p), p);
AsPropertyTypeDeserializer does p.nextToken() once and now is pointing at the first token of the temporary TokenBuffer built to keep track of the read tokens. So it's pointing at FIELD_NAME.

Since the concatenation starts with "fieldName" and "values", it is never starting with START_OBJECT, so the BeanDeserializer.deserialize will then attempt to _deserializeOther, while the json parser is now pointing at the FIELD_NAME

Then while going to BeanDeserializerBase.deserializeFromObjectUsingNonDefault(p, ctxt), delegateDeser is resolved as FromStingDeserializer, which will now be called... with a FIELD_NAME !

protected Object deserializeFromObjectUsingNonDefault(JsonParser p,
DeserializationContext ctxt) throws IOException
{
// 02-Jul-2024, tatu: [databind#4602] Need to tweak regular and "array" delegating
// Creator handling
final JsonDeserializer<Object> delegateDeser = _delegateDeserializer(p);
if (delegateDeser != null) {
final Object bean = _valueInstantiator.createUsingDelegate(ctxt,
delegateDeser.deserialize(p, ctxt));

FromStringDeserializer.deserialize never tries to figure out that it's not pointing at the right location...
So instead of using https://localhost to convert it into an URL, it uses the previous token...
So it tries to convert url (the field name) into an URL, which of course fails.

I'd suppose that FromStringDeserializer.deserialize should do :

if (p.currentToken() == JsonToken.FIELD_NAME) {
  p.nextToken();
}

at the beginning of the method :

public T deserialize(JsonParser p, DeserializationContext ctxt) throws IOException
{
// Let's get textual value, possibly via coercion from other scalar types
String text = p.getValueAsString();
if (text == null) {
JsonToken t = p.currentToken();
if (t != JsonToken.START_OBJECT) {
return (T) _deserializeFromOther(p, ctxt, t);
}
// 29-Jun-2020, tatu: New! "Scalar from Object" (mostly for XML)
text = ctxt.extractScalarFromObject(p, this, _valueClass);
}
if (text.isEmpty()) {
// 09-Jun-2020, tatu: Commonly `null` but may coerce to "empty" as well
return (T) _deserializeFromEmptyString(ctxt);
}
if (_shouldTrim()) {
final String old = text;
text = text.trim();
if (text != old) {
if (text.isEmpty()) {
return (T) _deserializeFromEmptyString(ctxt);
}
}
}

But I don't want to overthink it, and you might know of any better solution rather than walking around the code mistake.

Version Information

2.17.2

Reproduction

<-- Any of the following

  1. Brief code sample/snippet: include here in preformatted/code section
  2. Longer example stored somewhere else (diff repo, snippet), add a link
  3. Textual explanation: include here
    -->
// Your code here

Here is the definition for the HttpUri data class:

@CustomAnnotation
class HttpURI internal constructor(private val url: URL) : URI {
    override val scheme: String = url.protocol
    override val authority: String? = url.authority
    override val path: String = url.path
    val relativePath = "$path${url.query.prefixOrEscape("?")}${url.ref.prefixOrEscape("#")}"
    private val value = value()
    override fun value() = url.toString()
}

@CustomAnnotation
interface URI {
    val scheme: String
    val authority: String?
    val path: String
    fun value() = "$scheme:${authority.prefixOrEscape("://")}$path"
}

(the type info is defined using a JacksonAnnotationIntrospector for @CustomAnnotation, rather than using standard annotations)

here are the operations defined in the custom JacksonAnnotationInspector :

List<NamedType> subTypes =
        singletonList(new NamedType(HttpURI.class, "HttpURI"));
// there are 271 classes in reality, but I'm leaving only HttpUri in this snippet.

mapper.setAnnotationIntrospector(new JacksonAnnotationIntrospector() {
  @Override
  public List<NamedType> findSubtypes(Annotated ann) {
      return Stream.concat(subTypes.stream(),
              ofNullable(super.findSubtypes(ann)).orElse(emptyList()).stream()).collect(Collectors.toList());
  }

  @Override
  public JsonTypeInfo.Value findPolymorphicTypeInfo(MapperConfig<?> config, Annotated ann) {
      if (ann.getRawType().getAnnotation(CustomAnnotation.class) != null &&
              !(ann.getRawType().isInterface() || Modifier.isAbstract(ann.getRawType().getModifiers())) &&
              ann.getRawType() != RootLibraryElement.class)
          return jsonTypeInfo;
      return super.findPolymorphicTypeInfo(config, ann);
  }
});

here is a unit test :

    @Test
    void testDeserializeDesignSystemElements1() throws MalformedURLException, JsonProcessingException {
        ObjectMapper objectMapper = myObjectMapper();

        String response =
                "{\"url\":\"https://localhost\",\"$$element\":\"HttpURI\",\"scheme\":\"https\",\"authority\":\"localhost\",\"path\":\"\",\"relativePath\":\"\",\"value\":\"https://localhost\"}";


        URI element = objectMapper.readValue(response, HttpURI.class); // failing
        Assertions.assertEquals(
                "<HttpURI authority={\"localhost\"} path={\"\"} relativePath={\"\"} scheme={\"https\"} url={\"https://localhost\"} value={\"https://localhost\"} />",
                element.toString());
    }

(toString is doing a conversion from object to JSX format for React)

error message was :

com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java.net.URL` from String "url": not a valid textual representation, problem: no protocol: url
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 40]

Expected behavior

The value https://localhost should be parsed as URL instead of the field name url

Additional context

Here is the workaround I'm adding. That's really not a great situation to work with :

BeanDeserializerModifier beanDeserializerModifier = new BeanDeserializerModifier() {
        @Override
        public JsonDeserializer<?> modifyDeserializer(
                DeserializationConfig config,
                BeanDescription beanDesc,
                JsonDeserializer<?> deserializer) {
            if (deserializer instanceof FromStringDeserializer<?>) {
                return new JsonDeserializer<Object>() {
                    @Override
                    public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
                        // BEGINNING OF WORKAROUND
                        if (p.currentToken() == JsonToken.FIELD_NAME) {
                            p.nextToken();
                        }
                        // END OF WORKAROUND
                        return StdKeyDeserializer.forType(beanDesc.getBeanClass())
                                .deserializeKey(p.getText(), ctxt);
                    }
                };
            }
            return super.modifyDeserializer(config, beanDesc, deserializer);
        }
    };

    Module changeDeserializersModule = new SimpleModule("change-deserializers")
            .setDeserializerModifier(beanDeserializerModifier);
    mapper.registerModule(changeDeserializersModule)
@libetl libetl added the to-evaluate Issue that has been received but not yet evaluated label Jul 28, 2024
@libetl
Copy link
Contributor Author

libetl commented Aug 13, 2024

Hello @cowtowncoder ? How are you

@libetl
Copy link
Contributor Author

libetl commented Aug 13, 2024

I'm not expecting you to remember me but we worked on #1295 long time ago.

@cowtowncoder
Copy link
Member

Hi there! Apologies for slow follow-up: just returned from my 2-week vacation.

This seems quite involved; hoping to look into it soon.

@libetl
Copy link
Contributor Author

libetl commented Aug 13, 2024

No worries. Thank you for acknowledging this. Take your time

@cowtowncoder
Copy link
Member

Ok; quick note: FromStringDeserializer should not be given JsonToken.FIELD_NAME to read from -- if it is, that should thrown an exception.
Only BeanDeserializer and MapDeserializer should be expected to handle these (and similar JSON Object reading ones).

But it sounds more like either TokenBuffer should be constructed with starting JsonToken.START_OBJECT, or, more likely, code that accepts START_OBJECT should also work with FIELD_NAME (assuming missing START_OBJECT).
This is how BeanDeserializer generally works if my memory serves me.

@cowtowncoder
Copy link
Member

Ok I think HttpUri is in... Kotlin? I'd need Java version. Ideally it'd be great to have complete unit test and maybe I could see where things go wrong.
Without reproduction I am not sure I can help.

@libetl
Copy link
Contributor Author

libetl commented Aug 25, 2024

Hi !
I don't even need kotlin or any special config to reproduce the error,
just a scratch file in IntelliJ can do the trick.

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Assertions;

import java.net.MalformedURLException;
import java.net.URL;

class Scratch {
    @JsonTypeInfo(
            use = JsonTypeInfo.Id.SIMPLE_NAME,
            include = JsonTypeInfo.As.PROPERTY,
            property = "$$element",
            visible = false
    )
    public @interface CustomAnnotation {

    }

    public static void main(String[] args) throws MalformedURLException, JsonProcessingException {
        new Scratch().testDeserializeDesignSystemElements1();
        ;
    }

    @CustomAnnotation
    static class HttpURI implements Scratch.URI {
        private final URL url;

        @JsonCreator
        public HttpURI(URL url) {
            this.url = url;
        }

        public String getScheme() {
            return url.getProtocol();
        }

        public String getAuthority() {
            return url.getAuthority();
        }

        public String getPath() {
            return url.getPath();
        }

        public String getRelativePath() {
            return getPath() + url.getQuery().replaceAll("\\?", "\\?") +
                    url.getRef().replaceAll("#", "\\#");
        }

        public String getValue() {
            return url.toString();
        }
    }

    @CustomAnnotation
    interface URI {
        public String getScheme();

        public String getAuthority();

        public String getPath();

        default String getValue() {
            return getScheme() + ":" + getAuthority().replaceAll("/", "\\/") + getPath();
        }
    }

    void testDeserializeDesignSystemElements1() throws MalformedURLException, JsonProcessingException {
        ObjectMapper objectMapper = new ObjectMapper();

        String response =
                "{\"url\":\"https://localhost\",\"$$element\":\"HttpURI\"}";


        URI element = objectMapper.readValue(response, HttpURI.class); // failing
// Caused by: java.net.MalformedURLException: no protocol: url
// 	at java.base/java.net.URL.<init>(URL.java:674)
// 	at java.base/java.net.URL.<init>(URL.java:569)
// 	at java.base/java.net.URL.<init>(URL.java:516)
// 	at com.fasterxml.jackson.databind.deser.std.FromStringDeserializer$Std._deserialize(FromStringDeserializer.java:319)
        Assertions.assertInstanceOf(HttpURI.class, element);
    }
}

@cowtowncoder
Copy link
Member

How does @CustomAnnotation work here?

@JooHyukKim
Copy link
Member

JooHyukKim commented Aug 25, 2024

CustomAnnotation seems to be intended to work as a wrapper for @JsonTypeInfo.
See

    @JsonTypeInfo(
            use = JsonTypeInfo.Id.SIMPLE_NAME,
            include = JsonTypeInfo.As.PROPERTY,
            property = "$$element",
            visible = false
    )
    public @interface CustomAnnotation {

    }

in above example

@cowtowncoder
Copy link
Member

@JooHyukKim Conceptually that'd make sense, but nothing in code seems to make that happen. Maybe it's missing @JacksonAnnotationsInside or something. Test case seems incomplete, or maybe I am missing something.

@libetl
Copy link
Contributor Author

libetl commented Aug 26, 2024

So I can apparently reduce the entropy to be just this :

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Assertions;

import java.net.MalformedURLException;
import java.net.URL;

class Scratch {

    public static void main(String[] args) throws MalformedURLException, JsonProcessingException {
        new Scratch().testDeserializeDesignSystemElements();
    }

    static class HttpURI {
        private final URL url;

        @JsonCreator
        public HttpURI(URL url) {
            this.url = url;
        }
    }

    void testDeserializeDesignSystemElements() throws MalformedURLException, JsonProcessingException {
        ObjectMapper objectMapper = new ObjectMapper();

        String response = "{\"url\":\"https://localhost\"}";


        HttpURI element = objectMapper.readValue(response, HttpURI.class); // failing
        Assertions.assertInstanceOf(HttpURI.class, element);
    }
}

When I use @JsonProperty("url") in front of the URL constructor parameter, it works again.
Maybe that type inference as URL should not work and we should just throw an exception ?

@libetl
Copy link
Contributor Author

libetl commented Aug 26, 2024

It might not be related to JsonSubTypes at all.

@JooHyukKim
Copy link
Member

Seems to be wrong usage of @JsonCreator please refer to the documentation.
Here is a snippet of it.

image

@libetl
Copy link
Contributor Author

libetl commented Aug 26, 2024

so jackson tries to read it as a @JsonValue automatically ?
If I had two constructor parameters JsonCreator would have read it differently ?

@JooHyukKim
Copy link
Member

I am sorry, but I am not that familiar with behaviors wrt @JsonCreator behaviors.
What I meant was that if your case works with @JsonValue (like you said above), then it could be working as expected and I suggest you refer to @JsonCreator documentation.

And lemme know what you find!
Cheers

@libetl
Copy link
Contributor Author

libetl commented Aug 27, 2024

I don't want to get it parsed as @JsonValue.
So I think I'm missing a @JsonProperty in the constructor.
Thank you.

@libetl libetl closed this as completed Aug 27, 2024
@cowtowncoder
Copy link
Member

No, unless we are talking about an Enum value.
@JsonValue does not affect reading (deserialization) except for case of Enums; reference mentioned just suggests that if serialization (writing) uses @JsonValue, then typically @JsonCreator with Delegating mode is used.

But it is indeed very likely you are missing @JsonProperty -- alternatively you could have added

@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)

so Jackson didn't guess, incorrectly, that you want "DELEGATING" mode (it guess this way due to heuristics based and due to there not being getter or public field "url").
That is; for single-argument there are 2 possible JSONs that could match POJO with single Constructor argument:

"http://url"

{ "url" : "http://url" }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

3 participants