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

FrozenError - can't modify frozen String ... in: `force_encoding` #1478

Open
programmarchy opened this Issue Sep 21, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@programmarchy

programmarchy commented Sep 21, 2018

I encountered this error with ruby 2.5.1 and sinatra 2.0.3. The full stacktrace is below, but here's the culprit in sinatra/base.rb:

    # Fixes encoding issues by
    # * defaulting to UTF-8
    # * casting params to Encoding.default_external
    #
    # The latter might not be necessary if Rack handles it one day.
    # Keep an eye on Rack's LH #100.
    def force_encoding(*args) settings.force_encoding(*args) end
    if defined? Encoding
      def self.force_encoding(data, encoding = default_encoding)
        return if data == settings || data.is_a?(Tempfile)
        if data.respond_to? :force_encoding
          data.force_encoding(encoding).encode! # tries to mutate a frozen string
        elsif data.respond_to? :each_value
          data.each_value { |v| force_encoding(v, encoding) }
        elsif data.respond_to? :each
          data.each { |v| force_encoding(v, encoding) }
        end
        data
      end
    else
      def self.force_encoding(data, *) data end
    end

Here's a fix for that line:

          data.dup.force_encoding(encoding).encode!
2018-09-21 10:32:27 - FrozenError - can't modify frozen String:
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1757:in `force_encoding'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1757:in `force_encoding'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1759:in `block in force_encoding'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1759:in `each_value'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1759:in `force_encoding'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1752:in `force_encoding'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1092:in `dispatch!'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:924:in `block in call!'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1076:in `block in invoke'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1076:in `catch'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1076:in `invoke'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:924:in `call!'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:913:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/warden-1.2.7/lib/warden/manager.rb:36:in `block in call'
	/Users/Donald/.gem/ruby/2.5.1/gems/warden-1.2.7/lib/warden/manager.rb:35:in `catch'
	/Users/Donald/.gem/ruby/2.5.1/gems/warden-1.2.7/lib/warden/manager.rb:35:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/session/abstract/id.rb:232:in `context'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/session/abstract/id.rb:226:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/xss_header.rb:18:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/path_traversal.rb:16:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/json_csrf.rb:26:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/base.rb:50:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/base.rb:50:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/frame_options.rb:31:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/null_logger.rb:9:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/head.rb:12:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:194:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1958:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1502:in `block in call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1729:in `synchronize'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1502:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/warden-1.2.7/lib/warden/manager.rb:143:in `call_failure_app'
	/Users/Donald/.gem/ruby/2.5.1/gems/warden-1.2.7/lib/warden/manager.rb:129:in `process_unauthenticated'
	/Users/Donald/.gem/ruby/2.5.1/gems/warden-1.2.7/lib/warden/manager.rb:44:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/session/abstract/id.rb:232:in `context'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/session/abstract/id.rb:226:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/xss_header.rb:18:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/path_traversal.rb:16:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/json_csrf.rb:26:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/base.rb:50:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/base.rb:50:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-protection-2.0.3/lib/rack/protection/frame_options.rb:31:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/null_logger.rb:9:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/head.rb:12:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:194:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1958:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1502:in `block in call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1729:in `synchronize'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:1502:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/tempfile_reaper.rb:15:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/lint.rb:49:in `_call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/lint.rb:37:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/show_exceptions.rb:23:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/common_logger.rb:33:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/sinatra-2.0.3/lib/sinatra/base.rb:231:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/chunked.rb:54:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/content_length.rb:15:in `call'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/connection.rb:86:in `block in pre_process'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/connection.rb:84:in `catch'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/connection.rb:84:in `pre_process'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/connection.rb:53:in `process'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/connection.rb:39:in `receive_data'
	/Users/Donald/.gem/ruby/2.5.1/gems/eventmachine-1.2.7/lib/eventmachine.rb:195:in `run_machine'
	/Users/Donald/.gem/ruby/2.5.1/gems/eventmachine-1.2.7/lib/eventmachine.rb:195:in `run'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/backends/base.rb:73:in `start'
	/Users/Donald/.gem/ruby/2.5.1/gems/thin-1.7.2/lib/thin/server.rb:162:in `start'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/handler/thin.rb:22:in `run'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/server.rb:297:in `start'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/lib/rack/server.rb:148:in `start'
	/Users/Donald/.gem/ruby/2.5.1/gems/rack-2.0.5/bin/rackup:4:in `<top (required)>'
	/Users/Donald/.gem/ruby/2.5.1/bin/rackup:23:in `load'
	/Users/Donald/.gem/ruby/2.5.1/bin/rackup:23:in `<top (required)>'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/cli/exec.rb:74:in `load'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/cli/exec.rb:74:in `kernel_load'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/cli/exec.rb:28:in `run'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/cli.rb:424:in `exec'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/cli.rb:27:in `dispatch'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/cli.rb:18:in `start'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/exe/bundle:30:in `block in <top (required)>'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/lib/bundler/friendly_errors.rb:124:in `with_friendly_errors'
	/Users/Donald/.gem/ruby/2.5.1/gems/bundler-1.16.3/exe/bundle:22:in `<top (required)>'
	/Users/Donald/.gem/ruby/2.5.1/bin/bundle:23:in `load'
	/Users/Donald/.gem/ruby/2.5.1/bin/bundle:23:in `<main>'
::1 - - [21/Sep/2018:10:32:27 -0600] "POST /auth/login HTTP/1.1" 500 30 1.2399
@dentarg

This comment has been minimized.

Show comment
Hide comment
@dentarg

dentarg Sep 21, 2018

do you mind sharing how to reproduce the bug?

dentarg commented Sep 21, 2018

do you mind sharing how to reproduce the bug?

@programmarchy

This comment has been minimized.

Show comment
Hide comment
@programmarchy

programmarchy Sep 21, 2018

I'll need some time to distill the exact problem, but here's one way:

git clone git@github.com:simonneutert/sinatras-skeleton.git
cd sinatras-skeleton
bundle install
rake db:setup
bundle exec rackup

Go to http://localhost:9292/auth/login

Type in test / test for username / password.

You should get an internal server error, and the stack trace in my original comment.

programmarchy commented Sep 21, 2018

I'll need some time to distill the exact problem, but here's one way:

git clone git@github.com:simonneutert/sinatras-skeleton.git
cd sinatras-skeleton
bundle install
rake db:setup
bundle exec rackup

Go to http://localhost:9292/auth/login

Type in test / test for username / password.

You should get an internal server error, and the stack trace in my original comment.

@programmarchy

This comment has been minimized.

Show comment
Hide comment
@programmarchy

programmarchy Sep 21, 2018

@dentarg

Here's a simple example:

# frozen_string_literal: true
data = "hello world"
data.force_encoding('UTF-8')

I'm still not exactly sure how the frozen string is making its way to force_encode, but it's coming through the warden authenticate middleware.

programmarchy commented Sep 21, 2018

@dentarg

Here's a simple example:

# frozen_string_literal: true
data = "hello world"
data.force_encoding('UTF-8')

I'm still not exactly sure how the frozen string is making its way to force_encode, but it's coming through the warden authenticate middleware.

@dentarg

This comment has been minimized.

Show comment
Hide comment
@dentarg

dentarg Sep 21, 2018

@programmarchy That's not what I asked for. I want to know how the HTTP request look like that is causing the FrozenError to be raised.

If it only happens when the warden authenticate middleware is used, I'm not sure it's a legit Sinatra bug.

dentarg commented Sep 21, 2018

@programmarchy That's not what I asked for. I want to know how the HTTP request look like that is causing the FrozenError to be raised.

If it only happens when the warden authenticate middleware is used, I'm not sure it's a legit Sinatra bug.

@programmarchy

This comment has been minimized.

Show comment
Hide comment
@programmarchy

programmarchy Sep 21, 2018

@dentarg Still working on a specific test case.

Just pointing out the force_encode function is prone to crashing, so seems odd to blame the middleware at this point.

programmarchy commented Sep 21, 2018

@dentarg Still working on a specific test case.

Just pointing out the force_encode function is prone to crashing, so seems odd to blame the middleware at this point.

@programmarchy

This comment has been minimized.

Show comment
Hide comment
@programmarchy

programmarchy Sep 21, 2018

@dentarg

# frozen_string_literal: true

require 'sinatra/base'

class MyMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    req = Rack::Request.new(env)
    req.update_param('foo', 'bar')
    status, headers, response = @app.call(env)
    [status, headers, response]
  end
end

class MyApp < Sinatra::Base
  set :server, 'thin'

  use MyMiddleware

  get '/' do
    "hello"
  end
end

If the middleware instead does:

req.update_param('foo', 'bar'.dup)

Then it doesn't crash because 'bar' isn't frozen.

But how is the middleware supposed to know that Sinatra is going to mutate that param value?

programmarchy commented Sep 21, 2018

@dentarg

# frozen_string_literal: true

require 'sinatra/base'

class MyMiddleware
  def initialize(app)
    @app = app
  end

  def call(env)
    req = Rack::Request.new(env)
    req.update_param('foo', 'bar')
    status, headers, response = @app.call(env)
    [status, headers, response]
  end
end

class MyApp < Sinatra::Base
  set :server, 'thin'

  use MyMiddleware

  get '/' do
    "hello"
  end
end

If the middleware instead does:

req.update_param('foo', 'bar'.dup)

Then it doesn't crash because 'bar' isn't frozen.

But how is the middleware supposed to know that Sinatra is going to mutate that param value?

@dentarg

This comment has been minimized.

Show comment
Hide comment
@dentarg

dentarg Sep 21, 2018

Thanks, I see the same thing myself (https://github.com/dentarg/gists/tree/master/gists/sinatra-frozen)

I'm not really familiar with the Sinatra codebase, nor the rack codebase, so I don't have an answer for you.

Just gonna leave the Rack::Request#update_param documentation here, if it can give some clue to what's going on https://github.com/rack/rack/blob/2.0.5/lib/rack/request.rb#L363

Destructively update a parameter, whether it's in GET and/or POST. Returns nil.

dentarg commented Sep 21, 2018

Thanks, I see the same thing myself (https://github.com/dentarg/gists/tree/master/gists/sinatra-frozen)

I'm not really familiar with the Sinatra codebase, nor the rack codebase, so I don't have an answer for you.

Just gonna leave the Rack::Request#update_param documentation here, if it can give some clue to what's going on https://github.com/rack/rack/blob/2.0.5/lib/rack/request.rb#L363

Destructively update a parameter, whether it's in GET and/or POST. Returns nil.

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