also add Rational to this list? (1/1).to_r

What's the motivation? Fully testing special consts?
If so, it's the same complexity to have that as I mentiond. We need to add more examples such as BigDecimal, Float etc. I think it's not reasonable.
Otherwise, lmk.

I see, never mind then

np, thx

Can this logic be moved into the force_encoding method or its own method?

force_encoding should only be responsible for force encoding. I was thinking about moving it into its own method, like dup..., but I couldn't imagine that the change leads to bring happiness to us. Can you elaborate about the motivation?

I think extracting a method would help explain the need for the slightly unusual looking code. This all started to avoid a FrozenError right? The code doesn't communicate that. I'm not entirely sure if I understand the force_encoding method but take a look at this line:

It looks like the root issue is that a frozen string responds to force_encoding. I think the following would be a legitimate update to that method:

if data.respond_to? :force_encoding
  data = data.dup if data.frozen?
  data.force_encoding(encoding).encode!

I don't agree with the change on force_encoding because of two reasons.

1. force_encoding is only responsible for force encoding. In other words, it doesn't mean duping value, it should be managed in caller place, right? For example, force_encoding is also called in other places such as:

• sinatra/lib/sinatra/base.rb

  Line 1026 in 7b7043c

  force_encoding(params)

• sinatra/lib/sinatra/base.rb

  Line 1314 in 7b7043c

  force_encoding data, encoding

2. In general, our implementation does not always depend on the returned value. Using dup can only effect the returned value. Adopting your idea effects not only this issue, but other non-issue code. For example, a similar change with your idea had already been tested in Fix for FrozenError when force_encode a frozen string literal #1479, and got some failures.
   Of course, your code won't reproduce the failure, but the change has a possibility that leading the similar issue in the future.

For the above reasons, I wouldn't like to dup the value inside force_encoding.

I think extracting a method would help explain the need for the slightly unusual looking code.

Yeah I knew. But I couldn't imagine good name for that, and I don't think this is an unusual looking code. However, if you think so, we need to think about improving the readability.
I think we have two options for that:

1. Extracting method.
2. Adding comments

What do you think?

Shouldn't force_encoding guard or handle invalid scenarios? I think the fact that it is called in multiple locations only adds to that need. Are we sure that a FrozenError isn't a possible issue for them as well?

Anyway, I think a descriptive method name might be dup_frozen_params:

def dup_frozen_params(params)
  params.each do |key, val|
    if val.frozen?
      params[key] =
          begin
            val = val.dup
          rescue TypeError
            val
          end
    end
  end
end

@params.merge!(@request.params)
dup_frozen_params(@params)
force_encoding(@params)

I'm starting to wonder if this is necessary at all. What if we just changed the following line:

# This seems like a fair guard right?
if !data.frozen && data.respond_to? :force_encoding

Is there a need to force encode copies of frozen params? We could just leave it at that ;)

No, I did not say that.

If it is invalid to call force_encoding on a particular object, then I think we should account for that. Hence my last suggestion...a really simple fix. Don't dup at all and don't attempt to force_encode frozen strings.

# This seems like a fair guard right?
if !data.frozen? && data.respond_to? :force_encoding

Also, if that change is made it will be easy to write a method dup_frozen_params which could be called before attempting pass params to force_encoding. That was my main suggestion in that last post.

You meant not frozen but frozen?, right? As I said earlier, it's just skipping force_encoding. In other words, it can't force encoding when the value is frozen.
Why did you think the proposal is correct?
It's not same behavior with the current implementation and my proposal.

mock_app do
  use Class.new {
    def initialize(app)
      @app = app
    end

    def call(env)
      req = Rack::Request.new(env)
      req.update_param('bar', 'baz'.freeze)
      @app.call(env)
    end
  }
  set :default_encoding ,'ISO-8859-1'
  get '/' do
    params[:bar].encoding.to_s
  end
end
get '/'
assert_equal 'ISO-8859-1', body

As you can see, the value won't be forced encoding.

If it is invalid to call force_encoding on a particular object, then I think we should account for that.

Yes, exactly. It is not possible at this time to predict perfectly what kind of situation can be considered in the future, but it will be possible to create a method other than force_encoding or to make corrections at the caller as this time, for example.
At the very least, your proposal is focused on skipping exceptions, which is not at all rational.

Also, if that change is made it will be easy to write a method dup_frozen_params which could be called before attempting pass params to force_encoding. That was my main suggestion in that last post.

Again, ignoring frozen objects can not be a solution. It is not rational.

Ok forget that.

How about something like this for extracting a dup method:

    def dup_params!
      @params.each do |key, val|
        @params[key] = begin
         val.dup
        rescue TypeError
         val
        end
      end
    end

    # Dispatch a request with error handling.
    def dispatch!
      @params.merge!(@request.params)
      force_encoding(dup_params!)

What value of val causes #dup to raise TypeError? Played around in irb and couldn't come up with anything.

Ah, this is code for Ruby 2.3:

$ docker run --rm -it ruby:2.3.8-slim-jessie bash
root@f19c6072d969:/# irb
irb(main):001:0> nil.dup
TypeError: can't dup NilClass
	from (irb):1:in `dup'
	from (irb):1
	from /usr/local/bin/irb:11:in `<main>'

Such as true, false etc.. It can be reproduced in ruby <= 2.3.
See: https://github.com/ruby/ruby/blob/v2_3_7/object.c#L398