Skip to content

Support base64 encoding of bytes input for JSON blob types. - #397

Merged
danielgtaylor merged 2 commits into
boto:developfrom
danielgtaylor:json-bytes-blob
Dec 5, 2014
Merged

danielgtaylor merged 2 commits into
boto:developfrom
danielgtaylor:json-bytes-blob

Conversation

@danielgtaylor

Copy link
Copy Markdown
Contributor

This pull request modifies the code added in ffcef5f to also support
arguments passed in as bytes, such as those you might encounter when
using KMS.

See this related issue: boto/boto3#31

import boto3
kms = boto3.client('kms')
result = kms.encrypt(KeyId='some_key_id_redacted',Plaintext='my name is bob')
kms.decrypt(CiphertextBlob=result['CiphertextBlob'])

Since the JSON data for serialization tests doesn't support specifying
bytes as input, I've added a new test class to cover this case.

cc @jamesls, @kyleknap

This pull request modifies the code added in ffcef5f to also support
arguments passed in as bytes, such as those you might encounter when
using KMS.

See this related issue: boto/boto3#31

Since the JSON data for serialization tests doesn't support specifying
bytes as input, I've added a new test class to cover this case.
Comment thread botocore/serialize.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We are now no longer consistently handling blob types across protocols, as this logic is only in the JSONSerializer, and this is now duplicated from the query protocol. I think we should refactor this so that all the protocols can share a similar blob serializer. There's even a TODO note in the RestXML serializer specifically about this point.

@jamesls

jamesls commented Dec 4, 2014

Copy link
Copy Markdown
Member

+1 to handling this case, but we should port this over to all protocols so they all have the same behavior regarding bytes types.

This refactors the base64-encoding into a common method shared by
all serializers. The serializers now get the value using this new
method and just need to set it somewhere in the params.
@danielgtaylor

Copy link
Copy Markdown
Contributor Author

I've updated / refactored the code such that all serializers now use the same shared logic to convert a value to base64. Please take another look @jamesls!

@jamesls

jamesls commented Dec 5, 2014

Copy link
Copy Markdown
Member

:shipit: Looks good.

@kyleknap

kyleknap commented Dec 5, 2014

Copy link
Copy Markdown
Contributor

LGTM 🚢

danielgtaylor added a commit that referenced this pull request Dec 5, 2014
Support base64 encoding of bytes input for JSON blob types.
@danielgtaylor
danielgtaylor merged commit 32e33b6 into boto:develop Dec 5, 2014
@danielgtaylor
danielgtaylor deleted the json-bytes-blob branch December 5, 2014 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants