Search before asking

  • [x] I searched in the issues and found nothing similar.

Describe the bug

In Spring Security, we have a green unit test that can be reduced to:

JsonNode node = JsonMapper.builder().build().readTree("{\"username\":\"admin\",\"password\":null}");
assertThat(node.get("password").textValue()).isNull();

Looks basic, when I read a null String, I get null as expected.

With Jackson 3, this use case seems broken with both asString and stringValue: - node.get("password").asString() returns "" (empty string) - node.get("password").asString(null) also returns "" (empty string) - node.get("password").stringValue() fails with a 'NullNode' methodstringValue()cannot convert value <null> tojava.lang.String: value type not Stringexception - node.get("password").stringValue(null) return null but we can't use that as "{\"username\":\"admin\",\"password\":1}" would also return a null String. - node.get("password").asStringOpt() returns an Optional of "" (Optional of empty string)

I think the root issue here is not default value handling. The root issue here seems to be that on deserialization side, null is not considered as a valid value for JSON String, while in Jackson 2 it was and while in Jackson 3 serialization side it still is, as writeValueAsString(new User("admin", null)) write {"password":null,"username":"admin"} as expected.

I would expect Jackson 3 to allow me to get nullable string values when expected without having to rely on default values that are applicable to a lot of other use case as explained above, so not usable here.

The behavior I would expect is that Jackson 3 deserialization consider null as a valid String value so with "{\"username\":\"admin\",\"password\":null}": - node.get("password").asString() would return null instead of "" - node.get("password").stringValue() would return return null instead of "" - node.get("password").asStringOpt() would return an empty Optional instead of an Optional of "" (Optional of empty string)

Version Information

3.0.0-rc8

Reproduction

Expected behavior

No response

Additional context

No response

Comment From: cowtowncoder

Ok. Quick explanation of asXxx() (like "asString()"): these are methods that attempt conversion from various JSON input types into target types. For String target, typical coercion would indeed be empty String, "". If coercion not available, exception thrown (f.ex for JSON Object). Accessor xxxValue() (like "stringValue()") only returns target value if type is exactly the same: no coercion, but exception.

Variants that take argument "defaultValue" will return defaultValue in cases where exception would have been thrown. For stringValue() default value is returned for anything other than JSON String for example. Same is true foropt variants: "empty" is returned instead of throwing exception.

This handling should be consistent across all Node types.

But Jackson 2.x had, for some reason, peculiar handling for NullNode.asString() variations:

    @Override
    public String asText(String defaultValue) { return defaultValue; }

    @Override public String asText() { return "null"; }

so, "asXxX()" sometimes coerced (for some types), for others not. Exceptions never thrown (since Jackson exceptions are checked and no throws clause originally declared.

I don't think I would want to change any of "asXxx()" handling; this is convenience method and most users would probably not except nulls being returned (except wrt maybe Jackson 2.x compatibility?).

But should NullNode.stringValue() return null? That's reasonable question. Not sure how I feel about that.

Comment From: JooHyukKim

Jackson 3 node.get("password").stringValue(null) return null but we can't use that as "{\"username\":\"admin\",\"password\":1}" would also return a null String.

This part seems trickiest to solve...

Comment From: sdeleuze

Let me recap: - JsonNode is a low level API. - null and "" have different semantics in both Java and JSON - null is a valid Java String value. - JSON supports null values. - Jackson 3 writes Java null strings as JSON null - Jackson 3 is unable to read JSON null as Java null for strings (Jackson 2 was) - stringValue(null) does not allow to differentiate {"password":null} from invalid inputs like {"password":1}

I think this is not symmetric between read/write, inconsistent and IMO very confusing, even without talking about Jackson 2 compatibility. It also blocks a very common use case, since at JsonNode low level, it is IMO critically important to allow differentiating {"password":1}, {"password":""} and {"password":null}, but Jackson 3 currently does not in reasonable way. I can workaround via String password = (passwordNode.asToken() == JsonToken.VALUE_NULL || passwordNode instanceof MissingNode ? null : passwordNode.stringValue()); but IMO users should not be forced to do this kind of things since Java String supports null values.

But should NullNode.stringValue() return null? That's reasonable question. Not sure how I feel about that.

Strong +1 for NullNode.stringValue() retuning null, that would unblock our use case and provide a reasonable path to other users to translate {"password":null} to the expect Java null String.

That said, I reiterate that keeping asString() returning empty String and asStringOpt() returning non empty Optional of "" would be confusing. I get your point about coercion, but IMO it should not apply here as null is a valid Java String value so there is no type conversion involved. So I reiterate my proposal to implement return null when reading {"password":null} for both asString() and stringValue() and have asStringOpt() returning an empty Optional for such use case.

Also from a nullability perspective, and based on my experience in the JSpecify working group and leading the null-safety effort on Spring side, I would argue that the direction to follow in terms of API design is not to avoid null, but rather embrace it to express the absence of value (especially on low level APIs like JsonNode) but make nullability explicit at API level. I know Jackson 3.0 will not express nullability with JSpecify yet, but IMO when that will be the case, declaring these methods as @Nullable String asString() and @Nullable String stringValue() will perfectly make sense given the fact that both Java and JSON supports nullable values.

Comment From: cowtowncoder

I disagree with some of the points: most specifically null should not be result of asString(), ever. This is what "asXxx()" methods are for; coercing as possible values into expected target type. Empty String is more natural representation than Java null, as well as safer.

But I am open to stringValue() returning null as legit value to expose (instead of exception). One can argue that there is no coercion/conversion in Java sense even if in JSON Token type is different.

Note, too, that accessors cannot cover all cases, but there is JsonNode.isNull() and similar for actual type detection so it is definitely possible to distinguish underlying JSON type.

Comment From: sdeleuze

stringValue() returning null in that case would already be a great improvement.

Comment From: cowtowncoder

stringValue() returning null in that case would already be a great improvement.

I'll file an issue for this so we can get this part tackled quickly, without having to decide yet on asString() (and variants).

-> #5287 (need to fill in, but filed ASAP)

Comment From: cowtowncoder

Ok, I spent some more time thinking about my own reasoning wrt set of changes for 3.x and try to summarize salient points. But not yet describe changes.

For background, see JSTEP-3 and esp.

  • https://github.com/FasterXML/jackson-databind/issues/4958 -- numeric access, JsonNode.intValue()
  • https://github.com/FasterXML/jackson-databind/issues/4991 -- non-numeric access, JsonNode.booleanValue()
  • https://github.com/FasterXML/jackson-databind/issues/5003 -- converting accessors like JsonNode.asString()

String vs Null in JSON vs Java

One thing I hadn't thought about enough is this: null and "String" types are different in JSON and in Java:

  1. In JSON, token-wise, String values are distinct/different from null tokens. But
  2. In Java null is of course a valid String value

In my view, JsonNode is "JSON-centric" abstraction, hence StringNode (in 2.x, TextNode) vs NullNode. And this is the background of null being "not String" for purposes of JsonNode.stringValue() (and requiring conversion for asString()).

But while JSON-centric, JsonNode API itself operates to bridge the gap from JSON to Java, so null handling is as much a question of how Java works as how JSON is expressed. I somehow missed this perspective.

"asString()" really should be "toString()" (but "that Was Already Taken")

Now: name "asString()" could imply simple cast (... for which I intended "stringValue()") -- whereas its intent is NOT just that; it is to add meaningful conversions (esp. String to Number/Boolean). Conceptually, in fact, name "toString()" would be better match: intent being that whatever may be converted safely into (JSON...) String will be changed so.

But since JsonNode.toString() is already defined -- that is, JDK Object.toString() defines its contract; and 2.x implements logic:

  1. Cannot (should not) throw exceptions (JDK invariant)
  2. Produces legit JSON serialization (Jackson invariant)

I didn't think this should be used/re-defined. Hence asString() (and asBoolean() etc). I think this may contribute to surprise at null-conversion.

Convenient "asXxx()" vs strict/non-converting "xxxValue()"

Besides this, I wanted strict separation b/w:

  1. Only return value if type matches, otherwise fail (or return default/empty Optional, for variants) -- "stringValue()", "intValue()"
  2. Try your best to convert, only fail if you can't -- "asString()", "asInt()"

and for (2) further, avoid Java nulls for the usual reasons.

Inability to configure

A practical challenge: in many other cases, we could just add JsonNodeFeature to allow choosing null-handling. Alas, this is not an option here, as JsonNode instances have no access to configuration of ObjectMapper that (indireclty) created them. So the default behavior is all we have.

Comment From: sdeleuze

Thanks for taking the time to explain the reasoning behind those changes.

String vs Null in JSON vs Java

I understand the perspective that conceptually in JSON null and string are distinct, but JSON is also more dynamically typed so I think when we map a JSON node to Java, we should not force mapping JSON string to Java String one to one, but rather map Java String to the union of JSON (string|null) since those are valid in Java.

And on write side, Jackson 3 already considers that Java String is a union of JSON (string|null) since writing a null Java String write JSON null so as I user, I was expecting at least one option to have consistency between read and write, and I think JsonNode.stringValue() returning null for NullNode will clearly help to have symmetry/consistency between read and write.

The question could have been raised for number types as well, but the API is using primitive types so that's less confusing. For those testing with JsonNode.isNull() will be more natural as the primitive type is a clear signal that null can not been returned.

"asString()" really should be "toString()" (but "that Was Already Taken")

That helps to understand the reasoning, thanks for sharing.

Inability to configure

Makes sense to not have configurability for such low level feature.

Comment From: cowtowncoder

One last (?) thing -- I also would be open to some more "special" accessor -- asStringNullable()? -- even at the expense of uniformity (breaking that). Strings are somewhat special type, after all.

But I'll first want to get #5287 handled since there's no disagreement.