Comment 46 for bug 1927677

Hi,

me and my colleagues were noticing that the patch protects against URLs starting with two slashes, with four slashes, and with more than four slashes. Nevertheless, the patch does not seem to work in case the URL starts with three slashes, e.g.:

http://vncproxy.my.domain.com///example.com/%2F..

We believe that the reason behind this behaviour is the changes that the URL undergoes when it is split and then unsplit. More specifically:

>>> urlparse.urlunsplit(urlparse.urlsplit('//example.com/%2F..'))
'//example.com/%2F..'
>>> urlparse.urlunsplit(urlparse.urlsplit('///example.com/%2F..'))
'/example.com/%2F..'
>>> urlparse.urlunsplit(urlparse.urlsplit('////example.com/%2F..'))
'//example.com/%2F..'

The first and the last case will be captured by the test introduced by the patch, while the middle case will be sent to the parent class method. This can be observed with both Python 3 and Python 2.

We think that the unsplitting of the url in the patch could be avoided. In the case of Python 3, the reason for which the method is used is to add a final slash to the "path" part of the URL, but this is nevertheless not needed for our purposes (if the URL passes the validity check performed here, the splitting and unsplitting of the URL will be anyway done by the parent class method). Moreover, using the output of the urlsplit + urlunsplit methods causes the aforementioned issue.

I will soon attach a proposal for a Python 2 and a Python 3 patch. Their checks on the URL are slightly different because they mimic the checks that are done by the parent class method in order to identify a redirection case (SimpleHTTPRequestHandler.send_head defined in SimpleHttpServer.py for Python2 and in http/server.py for Python 3). Please let me know if you are able to reproduce the same issue in your setup and what you think about this.