Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequence.makeString() returns empty string on failure. #13

Closed
mjmsmith opened this issue May 22, 2017 · 2 comments
Closed

Sequence.makeString() returns empty string on failure. #13

mjmsmith opened this issue May 22, 2017 · 2 comments

Comments

@mjmsmith
Copy link

mjmsmith commented May 22, 2017

In ByteSequence+Conversions.swift.

This behavior seems likely to cause silent errors, as opposed to throwing or returning an optional string.

As an example, I was trying to get the string representation of a BCrypt salt back out of the digest. My first attempt (totally wrong, I should have used Serializer) was this:

try BCrypt.Parser(hashBytes).parseSalt().bytes.makeString()

It returned "" rather than indicating failure.

@mjmsmith mjmsmith changed the title Sequence.makeString() in ByteSequence+Conversions.swift returns empty string on failure. Sequence.makeString() returns empty string on failure. May 22, 2017
@tanner0101
Copy link
Contributor

Because of how integral this function is to every package in the framework, I don't think it will be possible to change. A lot of code relies on this not throwing and not returning an optional.

I'd recommend asserting that the string != "" or creating your own extension on Sequence that throws.

I've nonetheless added this to the Vapor 3 wishlist. Thanks!

@mjmsmith
Copy link
Author

No problem, I should have just used an optional String initializer in the first place. Might be worth changing the method name at some point to discourage use. (I suspect I only used it because I tripped across it in the BCrypt source.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants