Sinatra 1.3.2 have a different value for params than 1.3.1 #452

Closed
kyzh opened this Issue Jan 11, 2012 · 5 comments

Projects

None yet

4 participants

@kyzh

This is related to the smart-proxy (http://theforeman.org/).
Considering the follwing piece of code:

  post "/dhcp/:network" do
    logger.debug "Trying to create a new reservation, we got: #{params.inspect}"
    begin
      content_type :json
      @server.addRecord(params)
    rescue Proxy::DHCP::Collision => e
      log_halt 409, e
    rescue Proxy::DHCP::AlreadyExists
      # no need to do anything
    rescue => e
      log_halt 400, e
    end
  end

On version 1.3.2, here is my debug output:

Trying to create a new reservation, we got:
 {"ip"=>"10.23.11.4", "filename"=>"pxelinux.0", "network"=>"10.23.11.010.23.11.0", "mac"=>"3c:d9:2b:52:8d:ff", "hostname"=>"testprovision.somewhere.bzh"}

Then, by downgrading to the version 1.3.1, we get correct value for the "network bit"

Trying to create a new reservation, we got: {"hostname"=>"testprovision.somewhere.bzh", "ip"=>"10.23.11.4", "filename"=>"pxelinux.0", "mac"=>"3c:d9:2b:52:8d:ff", "network"=>"10.23.11.0"}

Nothing changed appart of the sinatra version.
So this is related to a third party.
I will try to come up with a test case code that doesn't involve involve foreman/smart-proxy.

I had a look for similar issue, but couldn't find one.

@rkh
Sinatra member

Is it possible that network is a key both in the URI and a field in the form?

@ohadlevy

that's possible :) however I'm not sure that concatenating the strings is the behavior I would expect :)

@rkh
Sinatra member

No, I'm not saying that behavior is correct, I just spotted the issue, it's not too hard to fix. I will push a fix later.

@ohadlevy

Thanks! ++

@Phrogz

+1 This just bit me, breaking one of my sites and taking me 3 hours to figure out what was going on. Here's a pared-down example of what I was doing:

spoof.ru

require 'sinatra'
class SpoofTest < Sinatra::Application
  get '/' do
    spoof_request "/HAI", 'REQUEST_METHOD'=>'GET'
  end

  get '/:item' do
    params[:item].inspect + "\n"
  end

  helpers do
    def spoof_request(uri,env_modifications={})
      call(env.merge("PATH_INFO" => uri).merge(env_modifications))
    end
  end
end
run SpoofTest.new

In Action

phrogz$ thin -R spoof.ru start &
…

phrogz$ curl http://localhost:3000/?item=oh-
"oh-HAI"

When you spoof the request, the params from the old request come through. Moreover, params values with conflicting names get appended, not replaced.

As this is the second time that my 'spoof_request' method has broken and had to be rewritten to match new internals, I'd really, really appreciate it if you could make this a first-class included helper method with a guaranteed UI, instead of just posting a deep-internals workaround that sometimes breaks.

@rkh rkh closed this in e328934 Mar 7, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment