From 11284a63d48c84f1d60b5686d55cf8a9f8d64422 Mon Sep 17 00:00:00 2001 From: Unai Zalakain Date: Mon, 4 Nov 2013 00:34:11 +0100 Subject: [PATCH] Fixed #18314 -- Corrected request.build_absolute_uri() handling of paths starting with // ``HttpRequest.build_absolute_uri()`` now correctly handles paths starting with ``//``. ``WSGIRequest`` now doesn't remove all the leading slashes either, because ``http://test/server`` and http://test//server`` aren't the same thing (RFC2396). Thanks to SmileyChris for the initial patch. --- django/core/handlers/wsgi.py | 6 +++- django/http/request.py | 27 +++++++++++----- docs/releases/1.8.txt | 6 +++- tests/requests/tests.py | 63 +++++++++++++++++++++++++++++++++++- 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py index 846c48e867..88be527d65 100644 --- a/django/core/handlers/wsgi.py +++ b/django/core/handlers/wsgi.py @@ -92,7 +92,11 @@ class WSGIRequest(http.HttpRequest): path_info = '/' self.environ = environ self.path_info = path_info - self.path = '%s/%s' % (script_name.rstrip('/'), path_info.lstrip('/')) + # be careful to only replace the first slash in the path because of + # http://test/something and http://test//something being different as + # stated in http://www.ietf.org/rfc/rfc2396.txt + self.path = '%s/%s' % (script_name.rstrip('/'), + path_info.replace('/', '', 1)) self.META = environ self.META['PATH_INFO'] = path_info self.META['SCRIPT_NAME'] = script_name diff --git a/django/http/request.py b/django/http/request.py index ded4744a87..84457c5300 100644 --- a/django/http/request.py +++ b/django/http/request.py @@ -15,7 +15,7 @@ from django.http.multipartparser import MultiPartParser, MultiPartParserError from django.utils import six from django.utils.datastructures import MultiValueDict, ImmutableList from django.utils.encoding import force_bytes, force_text, force_str, iri_to_uri -from django.utils.six.moves.urllib.parse import parse_qsl, urlencode, quote, urljoin +from django.utils.six.moves.urllib.parse import parse_qsl, urlencode, quote, urljoin, urlsplit RAISE_ERROR = object() @@ -119,14 +119,25 @@ class HttpRequest(object): def build_absolute_uri(self, location=None): """ Builds an absolute URI from the location and the variables available in - this request. If no location is specified, the absolute URI is built on - ``request.get_full_path()``. + this request. If no ``location`` is specified, the absolute URI is + built on ``request.get_full_path()``. Anyway, if the location is + absolute, it is simply converted to an RFC 3987 compliant URI and + returned and if location is relative or is scheme-relative (i.e., + ``//example.com/``), it is urljoined to a base URL constructed from the + request variables. """ - if not location: - location = self.get_full_path() - if not absolute_http_url_re.match(location): - current_uri = '%s://%s%s' % (self.scheme, - self.get_host(), self.path) + if location is None: + # Make it an absolute url (but schemeless and domainless) for the + # edge case that the path starts with '//'. + location = '//%s' % self.get_full_path() + bits = urlsplit(location) + if not (bits.scheme and bits.netloc): + current_uri = '{scheme}://{host}{path}'.format(scheme=self.scheme, + host=self.get_host(), + path=self.path) + # Join the constructed URL with the provided location, which will + # allow the provided ``location`` to apply query strings to the + # base path as well as override the host, if it begins with // location = urljoin(current_uri, location) return iri_to_uri(location) diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 3e241aff27..50d27aceb8 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -188,7 +188,11 @@ Templates Requests and Responses ^^^^^^^^^^^^^^^^^^^^^^ -* ... +* ``WSGIRequest`` now respects paths starting with ``//``. + +* The :meth:`HttpRequest.build_absolute_uri() + ` method now handles paths + starting with ``//`` correctly. Tests ^^^^^ diff --git a/tests/requests/tests.py b/tests/requests/tests.py index cf90fb1f1b..3f30f75eea 100644 --- a/tests/requests/tests.py +++ b/tests/requests/tests.py @@ -10,7 +10,7 @@ from django.core.exceptions import SuspiciousOperation from django.core.handlers.wsgi import WSGIRequest, LimitedStream from django.http import (HttpRequest, HttpResponse, parse_cookie, build_request_repr, UnreadablePostError, RawPostDataException) -from django.test import SimpleTestCase, override_settings +from django.test import SimpleTestCase, RequestFactory, override_settings from django.test.client import FakePayload from django.test.utils import str_prefix from django.utils import six @@ -693,3 +693,64 @@ class HostValidationTests(SimpleTestCase): msg_suggestion2 % "invalid_hostname.com", request.get_host ) + + +class BuildAbsoluteURITestCase(SimpleTestCase): + """ + Regression tests for ticket #18314. + """ + + def setUp(self): + self.factory = RequestFactory() + + def test_build_absolute_uri_no_location(self): + """ + Ensures that ``request.build_absolute_uri()`` returns the proper value + when the ``location`` argument is not provided, and ``request.path`` + begins with //. + """ + # //// is needed to create a request with a path beginning with // + request = self.factory.get('////absolute-uri') + self.assertEqual( + request.build_absolute_uri(), + 'http://testserver//absolute-uri' + ) + + def test_build_absolute_uri_absolute_location(self): + """ + Ensures that ``request.build_absolute_uri()`` returns the proper value + when an absolute URL ``location`` argument is provided, and + ``request.path`` begins with //. + """ + # //// is needed to create a request with a path beginning with // + request = self.factory.get('////absolute-uri') + self.assertEqual( + request.build_absolute_uri(location='http://example.com/?foo=bar'), + 'http://example.com/?foo=bar' + ) + + def test_build_absolute_uri_schema_relative_location(self): + """ + Ensures that ``request.build_absolute_uri()`` returns the proper value + when a schema-relative URL ``location`` argument is provided, and + ``request.path`` begins with //. + """ + # //// is needed to create a request with a path beginning with // + request = self.factory.get('////absolute-uri') + self.assertEqual( + request.build_absolute_uri(location='//example.com/?foo=bar'), + 'http://example.com/?foo=bar' + ) + + def test_build_absolute_uri_relative_location(self): + """ + Ensures that ``request.build_absolute_uri()`` returns the proper value + when a relative URL ``location`` argument is provided, and + ``request.path`` begins with //. + """ + # //// is needed to create a request with a path beginning with // + request = self.factory.get('////absolute-uri') + self.assertEqual( + request.build_absolute_uri(location='/foo/bar/'), + 'http://testserver/foo/bar/' + )