Skip to content

Conversation

@danielgtaylor
Copy link
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.
Copy link
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
Copy link
Member

jamesls commented Dec 4, 2014

+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
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
Copy link
Member

jamesls commented Dec 5, 2014

:shipit: Looks good.

@kyleknap
Copy link
Contributor

kyleknap commented Dec 5, 2014

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