Remove the encoding system #65
Conversation
After much discussion it was agreed that the encoding system was out of place in this library. This commit removes it, and hopefully improves the general quality of the codebase as well. All encoding-related thunks and glue code have been removed, the way TestVectors work have been standardized, and any stray test vectors still lingering inside of the tests have been moved to test_vectors.rb Fixes #54
|
|
| @@ -20,7 +17,8 @@ | |||
| end | |||
|
|
|||
| it "raises on a nil public key" do | |||
| expect { Crypto::Box.new(nil, bobsk) }.to raise_error(Crypto::LengthError, /Public key was 0 bytes \(Expected 32\)/) | |||
| expect { Crypto::Box.new(nil, bobsk) }.to raise_error(NoMethodError) | |||
| pending "is a failed #to_s (NoMethodError) here sufficient?" | |||
There was a problem hiding this comment.
@namelessjon what do you feel about this?
There was a problem hiding this comment.
I think we should definitely wrap such an error in something more descriptive than a nil complaining.
|
Looks good. Much simpler interfaces on everything 😀 I think we should keep a |
| # @param encoding [String] string encoding format in which to encode the key | ||
| # | ||
| # @return [String] key encoded in the specified format | ||
| def to_s(encoding = :raw) |
There was a problem hiding this comment.
Removing this is going to break #inspect, which relies on #to_s(:hex) being valid, so you should change that line too. Perhaps to use the Util.bin2hex method I suggested. Also, to avoid the chance of leaking 8 bytes of actual key, we could do something like compute the 8 byte Blake2b digest of the whole key and show that?
This replaces previous NoMethodError #to_s exceptions with a TypeError informing users that the given class can't be coerced to a String with #to_str
|
@namelessjon I think that should cover all the remaining problems, except perhaps first hashing serializables before inspecting them |
|
@namelessjon does this look good to you? I'd suggest tabling the |
Remove the encoding system This makes things one hell of a lot simpler to work with. We do keep bin2hex/hex2bin though. We're not monsters.
|
I'll take that as a yes ;) |
|
Seemed like the shortest way to say it 😄 |
After much discussion it was agreed that the encoding system was out of place in
this library.
This commit removes it, and hopefully improves the general quality of the
codebase as well. All encoding-related thunks and glue code have been removed,
the way TestVectors work have been standardized, and any stray test vectors
still lingering inside of the tests have been moved to test_vectors.rb
Fixes #54