Skip to content

Handle nil and empty string cleanly #35

Merged
merged 1 commit into from Apr 11, 2013

2 participants

@Capncavedan

Updates AES encryptor #encrypt and #decrypt to return value as-is when value is nil or the empty string.

@jmazzi jmazzi commented on an outdated diff Apr 11, 2013
lib/crypt_keeper/provider/aes.rb
@@ -29,8 +29,9 @@ def initialize(options = {})
# Public: Encrypt a string
#
- # Returns a string
+ # Returns nil if passed nil, or a string; empty-string is returned as-is
@jmazzi
Owner
jmazzi added a note Apr 11, 2013

How about adding a note like nil and empty strings are not encryptable with AES. When they are encountered, the orignal plaintext is returned. Then Returns the original value or encrypted string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmazzi
Owner
jmazzi commented Apr 11, 2013

Looks good! Just a quick doc change needed.

@Capncavedan

@jmazzi you got it!

@jmazzi jmazzi commented on an outdated diff Apr 11, 2013
spec/provider/aes_spec.rb
- subject.encrypt 'string'
+ context "a non-empty string" do
+ let(:encrypted) do
+ subject.encrypt 'string'
+ end
+
+ specify { encrypted.should_not == 'string' }
+ specify { encrypted.should_not be_blank }
+ end
+
+ context "an empty string" do
+ let(:encrypted) do
+ subject.encrypt ''
+ end
+
+ specify { encrypted.should == '' }
end
@jmazzi
Owner
jmazzi added a note Apr 11, 2013

You can remove the non-empty context, it's already covered under #encrypt

@jmazzi
Owner
jmazzi added a note Apr 11, 2013

What i mean is, they don't have to be under that context. Just leave them top level :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmazzi jmazzi commented on an outdated diff Apr 11, 2013
spec/provider/aes_spec.rb
end
describe "#decrypt" do
- let(:decrypted) do
- subject.decrypt "MC41MDk5MjI2NjgxMDI1MDI2OmNyeXB0X2tlZXBlcjpPI/8dCqWXDMVj7Jqs\nuwf/\n"
+ context "a non-empty string" do
+ let(:decrypted) do
+ subject.decrypt "MC41MDk5MjI2NjgxMDI1MDI2OmNyeXB0X2tlZXBlcjpPI/8dCqWXDMVj7Jqs\nuwf/\n"
+ end
+
+ specify { decrypted.should == 'string' }
@jmazzi
Owner
jmazzi added a note Apr 11, 2013

This is already covered as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Capncavedan

Sure thing

@jmazzi jmazzi commented on an outdated diff Apr 11, 2013
spec/provider/aes_spec.rb
end
+
@jmazzi
Owner
jmazzi added a note Apr 11, 2013

Can you remove this new line? :trollface:

@jmazzi
Owner
jmazzi added a note Apr 11, 2013

I meant the one on line 65 :O

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmazzi
Owner
jmazzi commented Apr 11, 2013

After that new line is removed, squash it and it's good to merge

@Capncavedan

Un-newlined and squashed!

@jmazzi jmazzi merged commit 7dff402 into jmazzi:master Apr 11, 2013

1 check was pending

Details default The Travis build is in progress
@Capncavedan Capncavedan deleted the MerchantsBonding:feature-handle-nil-and-empty-string-cleanly branch Apr 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.