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

devise-authy modifies strings unsafely #85

Open
kevinelliott opened this Issue Mar 1, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@kevinelliott
Copy link

kevinelliott commented Mar 1, 2018

In preparation for using frozen string literals, the devise-authy gem needs to be updated to handle them properly.

❯❯❯ RUBYOPT="--enable-frozen-string-literal" bundle exec rails server
/Users/kevin/.rvm/gems/ruby-2.4.3@workfit/gems/bundler-1.16.1/lib/bundler/runtime.rb:84:in `rescue in block (2 levels) in require': There was an error while trying to load the gem 'devise-authy'. (Bundler::GemRequireError)
Gem Load Error is: can't modify frozen String

The value for users is that frozen literals reduce memory consumption (in some cases up to 30% depending on how heavy string usage is) and performance gain potential is there too.

The resolution in this case might be in the dependency gem httpclient and addressable, but also every file in the gem should make use of the frozen string literal comment.

When strings need to be mutated, there are ways to accomplish this through the use of +"" and "".dup.

@senekis senekis added the enhancement label Apr 10, 2018

@philnash

This comment has been minimized.

Copy link
Contributor

philnash commented Jul 2, 2018

Hey @kevinelliott,

Thanks for this report. I've just taken over maintaining the gem and I will make sure we are frozen string literal safe as soon as I can.

@philnash

This comment has been minimized.

Copy link
Contributor

philnash commented Jul 5, 2018

From investigation the files that appear to violate frozen string literals are part of the Rails 4 test application that is embedded in the spec directory. This shouldn't be affecting loading the gem within an application though.

I will continue to investigate what can be done here.

@philnash

This comment has been minimized.

Copy link
Contributor

philnash commented Oct 5, 2018

OK, after further investigation, I believe that this is caused by httpclient. I will look to add the frozen string literal comment to the library to signify that authy-devise is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.