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

THRIFT-4446: JSONProtocol Base64 Encoding: Do not trim padding on encode. #1463

Closed
wants to merge 1 commit into from

Conversation

license-fn
Copy link
Contributor

  • In the C# and .NET Core libraries, the JSONProtocol's Binary Encoding to Base64 trims padding from the user provided byte arrays before encoding into Base64. This behavior is incorrect, as the user provided data should be encoded exactly as provided. Otherwise, data may be lost.

  • Fixed by no longer trimming padding on encode. Padding must still be trimmed on decode, in accordance with the Base64 specification.

  • For example:

    • Before this patch, encoding the byte array [0x01, 0x3d, 0x3d] yields [0x01] upon decode. This is incorrect, as I should decode the exact data that I encoded.
    • After this patch, it yields [0x01, 0x3d, 0x3d], as expected.

- In the C# and .NET Core libraries, the JSONProtocol's Binary Encoding
to Base64 trims padding from the user provided byte arrays before
encoding into base64. This behaviour is incorrect, as the user provided
bytes should be encoded exactly as they are provided. Otherwise, data
may be lost.
- Fixed by no longer trimming padding on encode.
@license-fn license-fn changed the title JSONProtocol Base64 Encoding: Do not trim padding on encode. THRIFT-4446: JSONProtocol Base64 Encoding: Do not trim padding on encode. Jan 9, 2018
Copy link
Contributor

@jeking3 jeking3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given it passed all cross tests, approving.

@asfgit asfgit closed this in d066fa8 Jan 11, 2018
jeking3 pushed a commit to jeking3/thrift that referenced this pull request Mar 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants