Skip to content

fix json deserialize byte to string bug #8140

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

Merged
merged 4 commits into from
Oct 5, 2020
Merged

Conversation

hangc0276
Copy link
Contributor

Fix #7657

Motivation

In GenericJsonRecord.java, it deserialize byte to String.

public Object getField(String fieldName) {
        JsonNode fn = jn.get(fieldName);
        if (fn.isContainerNode()) {
            AtomicInteger idx = new AtomicInteger(0);
            List<Field> fields = Lists.newArrayList(fn.fieldNames())
                .stream()
                .map(f -> new Field(f, idx.getAndIncrement()))
                .collect(Collectors.toList());
            return new GenericJsonRecord(schemaVersion, fields, fn);
        } else if (fn.isBoolean()) {
            return fn.asBoolean();
        } else if (fn.isFloatingPointNumber()) {
            return fn.asDouble();
        } else if (fn.isBigInteger()) {
            if (fn.canConvertToLong()) {
                return fn.asLong();
            } else {
                return fn.asText();
            }
        } else if (fn.isNumber()) {
            return fn.numberValue();
        } else {
            return fn.asText();
        }
    }

Changes

Add check the jsonNode binary type and convert to binaryValue instead of asText.

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@zymap
Copy link
Member

zymap commented Sep 28, 2020

Thanks for the fix. Could you please add a test for that?

@hangc0276
Copy link
Contributor Author

Thanks for the fix. Could you please add a test for that?

@zymap Thanks for you feedback. I will add the test.

@hangc0276
Copy link
Contributor Author

I have add the test, please take a look, thanks. @sijie @codelipenghui @jiazhai @zymap

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie sijie added this to the 2.7.0 milestone Oct 2, 2020
@sijie sijie requested review from codelipenghui and jiazhai October 2, 2020 15:32
@sijie sijie merged commit 66c3733 into apache:master Oct 5, 2020
wolfstudy pushed a commit that referenced this pull request Oct 30, 2020
Fix #7657 

### Motivation
In `GenericJsonRecord.java`, it deserialize byte to String.
```
public Object getField(String fieldName) {
        JsonNode fn = jn.get(fieldName);
        if (fn.isContainerNode()) {
            AtomicInteger idx = new AtomicInteger(0);
            List<Field> fields = Lists.newArrayList(fn.fieldNames())
                .stream()
                .map(f -> new Field(f, idx.getAndIncrement()))
                .collect(Collectors.toList());
            return new GenericJsonRecord(schemaVersion, fields, fn);
        } else if (fn.isBoolean()) {
            return fn.asBoolean();
        } else if (fn.isFloatingPointNumber()) {
            return fn.asDouble();
        } else if (fn.isBigInteger()) {
            if (fn.canConvertToLong()) {
                return fn.asLong();
            } else {
                return fn.asText();
            }
        } else if (fn.isNumber()) {
            return fn.numberValue();
        } else {
            return fn.asText();
        }
    }
```
### Changes
Add check the jsonNode binary type and convert to binaryValue instead of `asText`.

(cherry picked from commit 66c3733)
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Nov 13, 2020
Fix apache#7657 

### Motivation
In `GenericJsonRecord.java`, it deserialize byte to String.
```
public Object getField(String fieldName) {
        JsonNode fn = jn.get(fieldName);
        if (fn.isContainerNode()) {
            AtomicInteger idx = new AtomicInteger(0);
            List<Field> fields = Lists.newArrayList(fn.fieldNames())
                .stream()
                .map(f -> new Field(f, idx.getAndIncrement()))
                .collect(Collectors.toList());
            return new GenericJsonRecord(schemaVersion, fields, fn);
        } else if (fn.isBoolean()) {
            return fn.asBoolean();
        } else if (fn.isFloatingPointNumber()) {
            return fn.asDouble();
        } else if (fn.isBigInteger()) {
            if (fn.canConvertToLong()) {
                return fn.asLong();
            } else {
                return fn.asText();
            }
        } else if (fn.isNumber()) {
            return fn.numberValue();
        } else {
            return fn.asText();
        }
    }
```
### Changes
Add check the jsonNode binary type and convert to binaryValue instead of `asText`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AUTO_CONSUME with JSON serialized byte array produces string not byte array
3 participants