Passing multiple URL-encoded slashes in request path breaks router

[TheStalwart]

So the use case of mine is a simple REST service that would take an urlencoded HTTP URL as a first path item and then some more path items as a parameters.

My code:

get '/:url/*.*' do
	"url: #{params['feed_url']} request['path']: #{request.path} URL: #{request.url}"
end

The result of executing a request: http://localhost:4567/http%3A%2F%2Fwww.example.com%2Fpath%2F/

is the following output: url: http: request['path']: /http%3A/www.example.com/path/ URL: http://localhost:4567/http%3A/www.example.com/path/

I have a similar code implemented in NodeJS's "http" module, and it can parse this kind of request with no issues:

  pathname: '/http%3A%2F%2Fwww.example.com%2Fpath%2F/asdf',
  path: '/http%3A%2F%2Fwww.example.com%2Fpath%2F/asdf?asdf',
  href: '/http%3A%2F%2Fwww.example.com%2Fpath%2F/asdf?asdf' }

Is this a router bug or am i doing something wrong?

[mwpastore]

Sinatra tries to protect your app from path traversal attacks by being very careful with URL-encoded request paths. You can make it work the way you want it to by disabling Rack::Protection, or just Rack::Protection::PathTraversal:

require 'sinatra'

disable :protection # or set :protection, :except=>:path_traversal

get '/:url/' do
  "url: #{params[:url]} request.path: #{request.path} URL: #{request.url}"
end

(Note that the param key is :url (not :feed_url) because you named the parameter :url. Top-level params can be accessed by either string keys or symbol keys; it is an "indifferent" hash.)

This type of information is typically passed in a request header or query parameter instead to avoid any conflicts with the router.

Another way to do it without disabling path traversal attack protection would be to use a regex (potentially describing a valid URI) or a splat:

get %r{/(.*)/} do
  "url: #{params[:captures].first}"
end

get '/*/' do
  "url: #{params[:splat]}"
end

(Note that Rack::Protection::PathTraversal may still mangle the captured URLs somewhat; in this case, by squashing repeated forward slashes.)

[TheStalwart]

You can make it work the way you want it to by disabling Rack::Protection

Thanks, i'll try it when if/when i use Sinatra again (hacked up my thing in pure Rack yesterday).

Note that the param key is :url (not :feed_url) because you named the parameter :url

I've tried lots of things, including renaming parameters, before submitting a bugreport, so code pasted is not 100% correct, but just to explain the problem i encountered.

Another way to do it without disabling path traversal attack protection would be to use a regex

I'm pretty sure i tried that, and it didn't work, so disabling protection must be the only way to receive the parameter untouched

Thanks for the response, and hopefully this issue gets indexed by Google for the next unlucky guy to encounter it

[mwpastore]

Another way to do it without disabling path traversal attack protection would be to use a regex

I'm pretty sure i tried that, and it didn't work, so disabling protection must be the only way to receive the parameter untouched

I tested both solutions above and they work fine, aside from the forward-slash-mangling issue I mentioned. If you try it again and run into more problems, let us know.

Anyway, happy to help!

[ibnesayeed]

Many services that work on URIs as their primary means of lookup (such as web archives, URL shorteners, proxies, page scrapers etc.) would prefer to pass URLs as the last part of their path rather than in a query parameter. There are both good and bad things about such an approach, but it is being used commonly. This reminds me of https://github.com/mholt/caddy/issues/1298#issuecomment-269106412 discussion which lead to a fix in the Caddy Server.

[ibnesayeed]

Recently, I used Sinatra for a demo application, where I had a similar situation. So, I ended up using request.env["REQUEST_PATH"] to extract the desired information, but I did not like that approach. At that time I was not aware of Rack::Protection::PathTraversal, which I could have tried using otherwise.

[dentarg]

Looks resolved :)