unquote_url_inplace shouldn't unquote in place (or exist?) about bjoern

[jonashaag]

I added the unquoting code because of trouble with some WSGI frameworks (maybe Django?).

The WSGI spec is really not useful for the issue because it neither dictates that you should unquote paths nor that you shouldn't. Some people think it should be, others don't.

So I copied "real" WSGI servers here: CherryPy, gevent, Tornado, mod_python all unquote.

I believe it's safe to unquote URLs twice (in bjoern and in the framework) -- or can you name some counter-examples?

[patricklucas]

I did a test of six WSGI servers to see how they handled this: bjoern, gevent, tornado 1.0.1, tornado 2.0, wsgiref, and gunicorn.

I created a sample app, then curled the following path: /my%20test/34.1%2C-49.7

Of the six servers, only Tornado 1.0.1 left the path urlencoded; the others all printed /my test/34.1,-49.7, including Tornado 2.0.

Code:

app.py:

def app(environ, start_response):
    body = environ['PATH_INFO'] + '\n'

    headers = [
        ('Content-Type', 'text/plain'),
        ('Content-Length', str(len(body)))
    ]

    start_response('200 OK', headers)

    return body

test_bjoern.py

import bjoern
from app import app

bjoern.run(app, '127.0.0.1', 1234)

test_gevent.py

from gevent.wsgi import WSGIServer
from app import app

WSGIServer(('127.0.0.1', 1234), app).serve_forever()

test_tornado.py

from tornado import ioloop, httpserver, wsgi
from app import app

httpserver.HTTPServer(wsgi.WSGIContainer(app)).listen(1234)
ioloop.IOLoop.instance().start()

test_wsgiref.py

from wsgiref import simple_server
from app import app

simple_server.make_server('127.0.0.1', 1234, app).serve_forever()

For gunicorn, I executed the command: gunicorn -w 1 -b 127.0.0.1:1234 app:app

[eklitzke]

Interesting; I guess bjoern is doing the right thing then, especially if it matches the behavior of wsgiref, which is obviously the reference WSGI implementation. That's what I guess for basing my knowledge on Tornado.

Jonas, what do you think about changing the path to be a const char *. If I understand the matter correctly, making it const means doing one extra memcpy per request. A memcpy should be pretty cheap, but on the other hand the only real advantage is silencing a compiler warning -- AFAIK the const violation doesn't actually cause any correctness issues.

[jonashaag]

The right way to fix this is use const char* in the signature and then pass it to unquote_url_inplace using (char*)path.

[jonashaag]

bump

[jonashaag]

minor compiler warning. closing.

[jonashaag]

449e381