fix(grpc-gateway): handle null values in payload

[ghost]

Summary

#10658

Problem: When using the grpc-gateway plugin, if there is a null in the payload of the client request, such as:

{
    "step_id": "INT.L3-222",
    "step_parent_id": "INT.L2-64",
    "name": null
}

an error will be thrown: bad argument #2 to 'encode' (string expected for field 'object_type', got userdata), causing Kong to return 500 to the client, which is an error because Kong did not handle this edge case.

Cause: In JSON, null is valid and legal. However, after this payload is decoded by cjson, null actually becomes cjson.null in Kong, and its data type is userdata, but the name is defined as a string type in the proto file.

Solution: Use pcall to wrap the pb.encode function call, record the error log, and convert the error into a common client parameter error.

Checklist

• The Pull Request has tests
• There's an entry in the CHANGELOG
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Full changelog

• [Implement ...]

Issue reference

Fix #[issue number]

[hanshuebner]

Please provide a useful summary in your pull request description. What is the problem, what is the cause, what is the solution. It can be short, but it needs to describe what the change is about.

[hanshuebner]

Please be specific about what has been fixed. "Fix an issue" is not helpful.

[ghost]

updated, PTAL

[ghost]

Please provide a useful summary in your pull request description. What is the problem, what is the cause, what is the solution. It can be short, but it needs to describe what the change is about.

updated

[hbagdi]

@hanshuebner Could you please cherry pick this change into EE?

[arunpk-zageno]

@sabertobihwy does the fix accepts the JSON payload with null? or it will just throw an error says that "Null in Json"?

[ghost]

does the fix accepts the JSON payload with null? or it will just throw an error says that "Null in Json"?

it does't convert null in JSON payload to a string value, and will return failed to encode payload.

[arunpk-zageno]

But that doesn't fix the actual problem right?
As per the suggestion from @attenuation #10658 (comment) and our expectation, the fix should allow null values irrespective of what client throws us.

[ghost]

the fix should allow null values irrespective of what client throws us.

The error occurred due to a type matching error in lua-protobuf. If you want to pass null to upstream after encoding, there should be other ways, including redefining the properties of the field in the proto file to allow the field to accept both string|null attributes.

[oowl]

@arunpk-zageno This PR has fit my suggestion. After patched this PR, Kong can not return 5xx error code, returning 400 error. 500 error means is Kong internal error, it will print a traceback in the error log, and this is not expected behavior. The correct behavior is Kong at a very early age finds this mistake and tells customers that are not allowed to input.

[arunpk-zageno]

Got it @attenuation , And thanks @sabertobihwy for the fix. Which version I could expect this fix?

[oowl]

I think it will release in 3.3

[arunpk-zageno]

Sorry for iterative question, when can we expect the release?

[oowl]

Expected time is May 15th