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

SmileParser.getValueAsString() FIELD_NAME bug (same for CBOR, Protobuf) #541

Closed
johhud1 opened this issue Dec 30, 2024 · 2 comments
Closed
Labels
Milestone

Comments

@johhud1
Copy link
Contributor

johhud1 commented Dec 30, 2024

I believe there is a bug in SmileParser#getValueAsString().

Comparing the method implementation in SmileParser

    @Override
    public String getValueAsString() throws IOException
    {
        // inlined 'getText()' for common case of having String
        if (_tokenIncomplete) {
            _tokenIncomplete = false;
            int tb = _typeAsInt;
            int type = (tb >> 5);
            if (type == 2 || type == 3) { // tiny & short ASCII
                return _decodeShortAsciiValue(1 + (tb & 0x3F));
            }
            if (type == 4 || type == 5) { // tiny & short Unicode
                return _decodeShortUnicodeValue(2 + (tb & 0x3F));
            }
            _finishToken();
        }
        if (_currToken == JsonToken.VALUE_STRING) {
            return _textBuffer.contentsAsString();
        }
        if (_currToken == null || _currToken == JsonToken.VALUE_NULL || !_currToken.isScalarValue()) {
            return null;
        }
        return getText();
    }

To the implementation in jackson-core UTF8StreamJsonParser.java

    public String getValueAsString() throws IOException
    {
        if (_currToken == JsonToken.VALUE_STRING) {
            if (_tokenIncomplete) {
                _tokenIncomplete = false;
                return _finishAndReturnString(); // only strings can be incomplete
            }
            return _textBuffer.contentsAsString();
        }
        if (_currToken == JsonToken.FIELD_NAME) {
            return currentName();
        }
        return super.getValueAsString(null);
    }

The output is different when invoking the two methods on tokens of type JsonToken.FIELD_NAME, whereas I would expect them to behave similarly ( returning the current token field name as a string ).

I have a fork with a simple test update that reproduces the issue and a naive fix that worked for my use case.
dabe16a
The updated test fails without the change in SmileParser.java .

This came to my attention while trying to use the OpenSearch java sdk with jackson and 'application/smile' encoding. The deserialized responses from opensearch contained only null properties after switching to Smile ObjectMapper. The usage in question is here https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/json/ObjectDeserializer.java#L179 ; but it appears the usage there is correct.

Please let me know if there's any more info or help I can provide; or if I'm misunderstanding or mistaken.

@cowtowncoder
Copy link
Member

Thank you for reporting this @johhud1 -- it does look like a bug. I'll have a look at PR.

@cowtowncoder cowtowncoder added 2.17 and removed 2.18 labels Jan 4, 2025
@cowtowncoder cowtowncoder changed the title SmileParser getValueAsString() FIELD_NAME bug SmileParser.getValueAsString() FIELD_NAME bug (same for CBOR, Protobuf) Jan 4, 2025
cowtowncoder added a commit that referenced this issue Jan 4, 2025
@cowtowncoder cowtowncoder modified the milestones: 2.17.3, 2.17.4 Jan 4, 2025
@cowtowncoder
Copy link
Member

Merged in 2.19; backported to 2.17(.4), 2.18(.3) branches since this seems like a safe fix
(not sure if there will be 2.17.4, but 2.18.3 will be released)

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

No branches or pull requests

2 participants