Skip to content

Commit

Permalink
Canonicalize all proxito slashes (#8028)
Browse files Browse the repository at this point in the history
* Canonicalize all proxito slashes

This is currently broken,
eg. this URL works:
https://docs.readthedocs.io/en/latest///index.html

* Only parse the path not the params

* Use geturl() instead of urlunparse
  • Loading branch information
ericholscher committed Mar 18, 2021
1 parent b3185cf commit 0b3d41b
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
11 changes: 10 additions & 1 deletion readthedocs/proxito/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
"""
import logging
import sys
import re
from urllib.parse import urlparse

from django.conf import settings
from django.shortcuts import render
from django.shortcuts import render, redirect
from django.utils.deprecation import MiddlewareMixin
from django.urls import reverse

Expand Down Expand Up @@ -173,6 +175,13 @@ def process_request(self, request): # noqa
if hasattr(ret, 'status_code'):
return ret

if '//' in request.path:
# Remove multiple slashes from URL's
url_parsed = urlparse(request.get_full_path())
clean_path = re.sub('//+', '/', url_parsed.path)
new_parsed = url_parsed._replace(path=clean_path)
return redirect(new_parsed.geturl())

log.debug('Proxito Project: slug=%s', ret)

# Otherwise set the slug on the request
Expand Down
45 changes: 45 additions & 0 deletions readthedocs/proxito/tests/test_redirects.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,48 @@ def test_proper_page_on_subdomain(self):
'https://project.dev.readthedocs.io/en/latest/test.html',
)

def test_slash_redirect(self):
host = 'project.dev.readthedocs.io'

url = '/en/latest////awesome.html'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp['Location'], '/en/latest/awesome.html',
)

url = '///en/latest////awesome.html'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp['Location'], '/en/latest/awesome.html',
)

url = '///en/latest////awesome///index.html'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp['Location'], '/en/latest/awesome/index.html',
)

url = '///en/latest////awesome///index.html?foo=bar'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp['Location'], '/en/latest/awesome/index.html?foo=bar',
)

url = '///en/latest////awesome///'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp['Location'], '/en/latest/awesome/',
)

# Don't change the values of params
url = '///en/latest////awesome///index.html?foo=bar//bas'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp['Location'], '/en/latest/awesome/index.html?foo=bar//bas',
)

0 comments on commit 0b3d41b

Please sign in to comment.