Skip to content

Commit

Permalink
Proxito: always add nginx internal path (#11080)
Browse files Browse the repository at this point in the history
* Proxito: always add nginx internal path

Currently, this allows serving files like:
https://docs.readthedocs.io/_/static/proxito-static/javascript/readthedocs-analytics.js

We should always add the internal path to avoid serving files over
incorrect paths.

* Test
  • Loading branch information
stsewd committed Feb 1, 2024
1 parent d9c4e3c commit 034f54d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
22 changes: 22 additions & 0 deletions readthedocs/proxito/tests/test_full.py
Original file line number Diff line number Diff line change
Expand Up @@ -1729,6 +1729,28 @@ def test_serve_static_files(self):
resp.headers["Cache-Tag"], "project,project:rtd-staticfiles,rtd-staticfiles"
)

@mock.patch(
"readthedocs.proxito.views.mixins.staticfiles_storage",
new=StaticFileSystemStorageTest(),
)
def test_serve_static_files_internal_nginx_redirect_always_appended(self):
"""Test for #11080."""
resp = self.client.get(
reverse(
"proxito_static_files",
args=["proxito-static/javascript/readthedocs-doc-embed.js"],
),
headers={"host": "project.readthedocs.io"},
)
self.assertEqual(resp.status_code, 200)
self.assertEqual(
resp.headers["x-accel-redirect"],
"/proxito-static/media/proxito-static/javascript/readthedocs-doc-embed.js",
)
self.assertEqual(
resp.headers["Cache-Tag"], "project,project:rtd-staticfiles,rtd-staticfiles"
)

@mock.patch("readthedocs.proxito.views.mixins.staticfiles_storage")
def test_serve_invalid_static_file(self, staticfiles_storage):
staticfiles_storage.url.side_effect = Exception
Expand Down
20 changes: 7 additions & 13 deletions readthedocs/proxito/views/mixins.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import copy
import mimetypes
from urllib.parse import parse_qsl, urlencode, urlparse

Expand Down Expand Up @@ -233,22 +232,17 @@ def _serve_file_from_nginx(self, path, root_path):
:param path: The path of the file to serve.
:param root_path: The root path of the internal redirect.
"""
original_path = copy.copy(path)
if not path.startswith(f"/{root_path}/"):
if path[0] == "/":
path = path[1:]
path = f"/{root_path}/{path}"

internal_path = f"/{root_path}/" + path.lstrip("/")
log.debug(
"Nginx serve.",
original_path=original_path,
path=path,
original_path=path,
internal_path=internal_path,
)

content_type, encoding = mimetypes.guess_type(path)
content_type, encoding = mimetypes.guess_type(internal_path)
content_type = content_type or "application/octet-stream"
response = HttpResponse(
f"Serving internal path: {path}", content_type=content_type
f"Serving internal path: {internal_path}", content_type=content_type
)
if encoding:
response["Content-Encoding"] = encoding
Expand All @@ -258,11 +252,11 @@ def _serve_file_from_nginx(self, path, root_path):
# as the header value.
# https://github.com/benoitc/gunicorn/issues/1448
# https://docs.djangoproject.com/en/1.11/ref/unicode/#uri-and-iri-handling
x_accel_redirect = iri_to_uri(path)
x_accel_redirect = iri_to_uri(internal_path)
response["X-Accel-Redirect"] = x_accel_redirect

# Needed to strip any GET args, etc.
response.proxito_path = urlparse(path).path
response.proxito_path = urlparse(internal_path).path
return response

def _serve_file_from_python(self, request, path, storage):
Expand Down

0 comments on commit 034f54d

Please sign in to comment.