interledger-deprecated / java-crypto-conditions

WE'VE MOVED: This project has moved to Hyperledger Quilt
https://github.com/hyperledger/quilt/tree/master/crypto-conditions
Apache License 2.0
5 stars 12 forks source link

Consider Removing Condtion.getFingerprintBase64Url #76

Closed sappenin closed 7 years ago

sappenin commented 7 years ago

See discussion in this PR: https://github.com/interledger/java-ilp-core/pull/102#issuecomment-337885767

The reason Condtion#getFingerprintBase64Url was introduced was to try and create a truly immutable implementation of each condition. However, this encoding is a bit ambiguous (e.g., is padding used, or not, etc), and while this ambiguity could be cleared-up in Javadoc, perhaps it's better to simply remove this method entirely, and return a copy of the bytes in any implementation of Condition#getFingerprint.

The downside of this is possibly performance, but as @adrianhopebailie indicated, these are only 32 bytes, and if somebody encounters a perf problem, we could potentially add a new method to just return a reference to the array. The reason we aren't doing that now is mutability -- returning a reference to the array would allow an external caller to mutate the array (though this is likely already possible via Reflection, so until Java introduces an immutable byte array, there's probably not much use in trying to guard against this -- unless we want to only store a String, but that has its own downsides due to potential encoding/decoding mistakes on the part of a developer).