Flag deprecated encryptString/decryptString APIs as deprecated. #120
Conversation
* Add @deprecated to deprecated APIs * Update Javadoc about deprecation * Opportunistic improvement of incomplete Javadoc on decryptData()
Per feedback from @juneb
| @@ -281,7 +281,12 @@ public long estimateCiphertextSize( | |||
| /** | |||
| * Calls {@link #encryptData(MasterKeyProvider, byte[], Map)} on the UTF-8 encoded bytes of | |||
| * {@code plaintext} and base64 encodes the result. | |||
| * @deprecated Use the {@link #encryptData(MasterKeyProvider, byte[], Map)} and | |||
| * {@link #decryptData(MasterKeyProvider, byte[])} APIs instead. {@code encryptString} is incompatible with the | |||
There was a problem hiding this comment.
I would not say that they are incompatible without clarification. Maybe "because the the output must be base64-decoded to be compatible with other AWS Encryption SDK implementations"?
There was a problem hiding this comment.
Works for me, I am but a deprecation-shaped vessel for the wordsmithing that you guys think make sense.
There was a problem hiding this comment.
Because these APIs return Base64-encoded output, they are not compatible with other AWS Encryption SDK language implementations.
There was a problem hiding this comment.
Because these APIs return Base64-encoded output, they are not compatible with other AWS Encryption SDK language implementations.
IMO this still conveys the wrong message.
What I would like to make sure we're communicating is that they are compatible, but only once you peal away an extra layer of encoding that this method adds.
There was a problem hiding this comment.
After some noodling, I've tried again to be clear and concise in explaining that this is deprecated, and why. When you have a second, please let me know what you think of the new wording.
|
@lizroth We should also add the deprecation message to the example that demonstrates how to use the encryptString and decryptString methods, or remove them. |
Agreed. Removed the example. |
Removing example code for deprecated functions. Original commit got lost in PR cleanup.
Issue #, if available: n/a
Description of changes:
@Deprecatedto deprecated APIsBackground: the
encryptString/decryptStringAPIs were an early approach to try to provide helpful encoding for string ciphertexts. However, the AWS Encryption SDKs in general attempt to be agnostic as to the data format that users want to use for their ciphertexts, so over the longer term these APIs have turned out to be more confusing than beneficial for customers. As such, most Encryption SDKs do not support these APIs by design.Only the Java ESDK has these APIs, because Java was the original implementation, and at the time it was a more significant convenience improvement to offer these APIs to Java developers.
This PR deprecates those APIs to make it clear to our users which APIs are interoperable and to encourage our users to prefer the ones that are.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: