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

Rack handles on-disk file and directory names with + signs in them. #271

Closed
wants to merge 2 commits into from

Conversation

VictorLowther
Copy link

I encountered this bug when trying to use Rainbows to serve up an Ubuntu install tree for net installs. Out of all the web servers I tried, only Rainbows failed to act as a backing webserver when using Rack::Directory to serve up the directory tree -- the install failed the first time it tried to serve a filename with a + in the name.

To replicate:
sudo gem install rack rainbows
mkdir -p test/foo+bar
echo "here"> test/foo+bar/bar+baz
echo 'run Rack::Directory.new(".")' >test/config.ru
(cd test; rainbows -p 8080)

Then, in a different terminal:
wget -O - --save-headers http://localhost:8080/
wget -O - --save-headers http://localhost:8080/foo+bar/
wget -O - --save-headers http://localhost:8080/foo+bar/bar+baz

wget will print a directory index for the first pull, and get 404 errors for the second and third. The Rainbows output will show:

127.0.0.1 - - [19/Nov/2011 09:27:45] "GET / HTTP/1.0" 200 - 0.0153
127.0.0.1 - - [19/Nov/2011 09:28:14] "GET /foo+bar/ HTTP/1.0" 404 28 0.0024
127.0.0.1 - - [19/Nov/2011 09:31:40] "GET /foo+bar/bar+baz HTTP/1.0" 404 35 0.0023

With the changes in this patch, I get:

127.0.0.1 - - [19/Nov/2011 09:39:55] "GET / HTTP/1.0" 200 - 0.0046
127.0.0.1 - - [19/Nov/2011 09:40:00] "GET /foo+bar/ HTTP/1.0" 200 - 0.0046
127.0.0.1 - - [19/Nov/2011 09:40:07] "GET /foo+bar/bar+baz HTTP/1.0" 200 - 0.0023

Webrick, nginx, thttpd, and apache all do the expected thing. The issue is not whether the client is failing to properly encode requests to the server (unless wget has managed to get it wrong for all these years). The issue is that Rack appears to be doing the Wrong Thing when decoding the requests and matching them against the files I am asking it to serve.

…/file name.file from the POV of the web server, which is a Bad Thing.
…/file name.file from the POV of the web server, which is a Bad Thing.
@hannesg
Copy link

hannesg commented Nov 20, 2011

Hi

The patch leads to 4 failing specs with urlescaped paths. Simply removing the unescaping is IMHO not 100% correct. I think it would be better to use a special unescaping method, which doesn't unescape +.

@kagminjeong
Copy link
Contributor

See #265 for the discussion.

@raggi
Copy link
Member

raggi commented Dec 5, 2011

Effectively closed by c701fc8.

I am sorry that this does not conform to your desired use case. See #265 for more information.

@raggi raggi closed this Dec 5, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants