From 16ae1abbc3b99879b687dd20c15fae7e05c03335 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Wed, 20 Dec 2023 11:21:59 +0100
Subject: [PATCH 01/10] Development: use `wrangler` locally (update
 NGINX/Dockerfile config)

Related https://github.com/readthedocs/addons/issues/217
---
 common                                   |  2 +-
 dockerfiles/Dockerfile.wrangler          |  2 ++
 dockerfiles/nginx/proxito.conf.template  | 13 ++-----------
 dockerfiles/nginx/wrangler.conf.template | 20 ++++++++++++++++++++
 4 files changed, 25 insertions(+), 12 deletions(-)
 create mode 100644 dockerfiles/Dockerfile.wrangler
 create mode 100644 dockerfiles/nginx/wrangler.conf.template

diff --git a/common b/common
index 6093983195e..e598102991c 160000
--- a/common
+++ b/common
@@ -1 +1 @@
-Subproject commit 6093983195ea4699169e7652635e437e603a93c1
+Subproject commit e598102991c5a927024f06f187f51f7000e67023
diff --git a/dockerfiles/Dockerfile.wrangler b/dockerfiles/Dockerfile.wrangler
new file mode 100644
index 00000000000..19c62a2d94b
--- /dev/null
+++ b/dockerfiles/Dockerfile.wrangler
@@ -0,0 +1,2 @@
+FROM node:18.15
+RUN npm install -g wrangler@3.21.0
diff --git a/dockerfiles/nginx/proxito.conf.template b/dockerfiles/nginx/proxito.conf.template
index 8b5699d1a2b..f1d950c7710 100644
--- a/dockerfiles/nginx/proxito.conf.template
+++ b/dockerfiles/nginx/proxito.conf.template
@@ -1,7 +1,7 @@
 # Proxito
 server {
-    listen 80 default_server;
-    server_name $NGINX_PROXITO_SERVER_NAME;
+    listen 8080;
+    server_name nginx;
 
     # Docker Compose's "logging.driver: none" is not working anymore.
     # So, we are disabling the logs from NGINX directly.
@@ -23,10 +23,8 @@ server {
     # Proxito doc serving
     location / {
         proxy_pass http://proxito:8000;
-        proxy_set_header Host $host;
         proxy_set_header X-Real-IP $remote_addr;
         proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
-        proxy_set_header X-Forwarded-Host $host;
 
         proxy_intercept_errors on;
         error_page 404 = @notfoundfallback;
@@ -110,13 +108,6 @@ server {
         add_header X-RTD-Hosting-Integrations $rtd_hosting_integrations always;
         set $rtd_force_addons $upstream_http_x_rtd_force_addons;
         add_header X-RTD-Force-Addons $rtd_force_addons always;
-
-        # Inject our own script dynamically and project/version slugs into the HTML to emulate what CF worker does
-        # TODO: find a way to make this work _without_ running `npm run dev` from the `addons` repository
-        sub_filter '</head>' '<script async language="javascript" src="http://localhost:8000/readthedocs-addons.js"></script>\n<meta name="readthedocs-project-slug" content="$rtd_project" />\n<meta name="readthedocs-version-slug" content="$rtd_version" />\n</head>';
-        sub_filter_types text/html;
-        sub_filter_last_modified on;
-        sub_filter_once on;
     }
 
     # Serve 404 pages here
diff --git a/dockerfiles/nginx/wrangler.conf.template b/dockerfiles/nginx/wrangler.conf.template
new file mode 100644
index 00000000000..b99d1faef1d
--- /dev/null
+++ b/dockerfiles/nginx/wrangler.conf.template
@@ -0,0 +1,20 @@
+# Proxito
+server {
+    listen 80 default_server;
+    server_name $NGINX_PROXITO_SERVER_NAME;
+
+    # Docker Compose's "logging.driver: none" is not working anymore.
+    # So, we are disabling the logs from NGINX directly.
+    access_log off;
+
+    # Wrangler serving
+    location / {
+        proxy_pass http://wrangler:8000;
+        proxy_set_header Host $host;
+        proxy_set_header X-Real-IP $remote_addr;
+        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+        proxy_set_header X-Forwarded-Host $host;
+
+        add_header X-Served CF-Wrangler always;
+    }
+}

From 802c76ca4b24d06250c911e66df9cd8610331270 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Tue, 6 Feb 2024 13:15:03 +0100
Subject: [PATCH 02/10] Add CF Worker .js script

---
 dockerfiles/force-readthedocs-addons.js | 219 ++++++++++++++++++++++++
 1 file changed, 219 insertions(+)
 create mode 100644 dockerfiles/force-readthedocs-addons.js

diff --git a/dockerfiles/force-readthedocs-addons.js b/dockerfiles/force-readthedocs-addons.js
new file mode 100644
index 00000000000..161ff58c494
--- /dev/null
+++ b/dockerfiles/force-readthedocs-addons.js
@@ -0,0 +1,219 @@
+/*
+
+  Script to inject the new Addons implementation on pages served by El Proxito.
+
+  This script is ran on a Cloudflare Worker and modifies the HTML with two different purposes:
+
+      1. remove the old implementation of our flyout (``readthedocs-doc-embed.js`` and others)
+      2. inject the new addons implementation (``readthedocs-addons.js``) script
+
+  Currently, we are doing 1) only when users opt-in into the new beta addons.
+  In the future, when our addons become stable, we will always remove the old implementation,
+  making all the projects to use the addons by default.
+
+*/
+
+// add "readthedocs-addons.js" inside the "<head>"
+const addonsJs =
+  '<script async type="text/javascript" src="/_/static/javascript/readthedocs-addons.js"></script>';
+
+// selectors we want to remove
+// https://developers.cloudflare.com/workers/runtime-apis/html-rewriter/#selectors
+const analyticsJs =
+  'script[src="/_/static/javascript/readthedocs-analytics.js"]';
+const docEmbedCss = 'link[href="/_/static/css/readthedocs-doc-embed.css"]';
+const docEmbedJs =
+  'script[src="/_/static/javascript/readthedocs-doc-embed.js"]';
+const analyticsJsAssets =
+  'script[src="https://assets.readthedocs.org/static/javascript/readthedocs-analytics.js"]';
+const docEmbedCssAssets =
+  'link[href="https://assets.readthedocs.org/static/css/readthedocs-doc-embed.css"]';
+const docEmbedJsAssets =
+  'script[src="https://assets.readthedocs.org/static/javascript/readthedocs-doc-embed.js"]';
+const docEmbedJsAssetsCore =
+  'script[src="https://assets.readthedocs.org/static/core/js/readthedocs-doc-embed.js"]';
+const badgeOnlyCssAssets =
+  'link[href="https://assets.readthedocs.org/static/css/badge_only.css"]';
+const badgeOnlyCssAssetsProxied = 'link[href="/_/static/css/badge_only.css"]';
+const readthedocsExternalVersionWarning = "[role=main] > div:first-child > div:first-child.admonition.warning";
+const readthedocsFlyout = "div.rst-versions";
+
+// "readthedocsDataParse" is the "<script>" that calls:
+//
+//   READTHEDOCS_DATA = JSON.parse(document.getElementById('READTHEDOCS_DATA').innerHTML);
+//
+const readthedocsDataParse = "script[id=READTHEDOCS_DATA]:first-of-type";
+const readthedocsData = "script[id=READTHEDOCS_DATA]";
+
+// do this on a fetch
+addEventListener("fetch", (event) => {
+  const request = event.request;
+  event.respondWith(handleRequest(request));
+});
+
+async function handleRequest(request) {
+  // perform the original request
+  let originalResponse = await fetch(request);
+
+  // get the content type of the response to manipulate the content only if it's HTML
+  const contentType = originalResponse.headers.get("content-type") || "";
+  const injectHostingIntegrations =
+    originalResponse.headers.get("x-rtd-hosting-integrations") || "false";
+  const forceAddons =
+    originalResponse.headers.get("x-rtd-force-addons") || "false";
+
+  // Log some debugging data
+  console.log(`ContentType: ${contentType}`);
+  console.log(`X-RTD-Force-Addons: ${forceAddons}`);
+  console.log(`X-RTD-Hosting-Integrations: ${injectHostingIntegrations}`);
+
+  // get project/version slug from headers inject by El Proxito
+  const projectSlug = originalResponse.headers.get("x-rtd-project") || "";
+  const versionSlug = originalResponse.headers.get("x-rtd-version") || "";
+
+  // check to decide whether or not inject the new beta addons:
+  //
+  // - content type has to be "text/html"
+  // when all these conditions are met, we remove all the old JS/CSS files and inject the new beta flyout JS
+
+  // check if the Content-Type is HTML, otherwise do nothing
+  if (contentType.includes("text/html")) {
+    // Remove old implementation of our flyout and inject the new addons if the following conditions are met:
+    //
+    // - header `X-RTD-Force-Addons` is present (user opted-in into new beta addons)
+    // - header `X-RTD-Hosting-Integrations` is not present (added automatically when using `build.commands`)
+    //
+    if (forceAddons === "true" && injectHostingIntegrations === "false") {
+      return (
+        new HTMLRewriter()
+          .on(analyticsJs, new removeElement())
+          .on(docEmbedCss, new removeElement())
+          .on(docEmbedJs, new removeElement())
+          .on(analyticsJsAssets, new removeElement())
+          .on(docEmbedCssAssets, new removeElement())
+          .on(docEmbedJsAssets, new removeElement())
+          .on(docEmbedJsAssetsCore, new removeElement())
+          .on(badgeOnlyCssAssets, new removeElement())
+          .on(badgeOnlyCssAssetsProxied, new removeElement())
+          .on(readthedocsExternalVersionWarning, new removeElement())
+          .on(readthedocsFlyout, new removeElement())
+          // NOTE: I wasn't able to reliably remove the "<script>" that parses
+          // the "READTHEDOCS_DATA" defined previously, so we are keeping it for now.
+          //
+          // .on(readthedocsDataParse, new removeElement())
+          // .on(readthedocsData, new removeElement())
+          .on("head", new addPreloads())
+          .on("head", new addProjectVersionSlug(projectSlug, versionSlug))
+          .transform(originalResponse)
+      );
+    }
+
+    // Inject the new addons if the following conditions are met:
+    //
+    // - header `X-RTD-Hosting-Integrations` is present (added automatically when using `build.commands`)
+    // - header `X-RTD-Force-Addons` is not present (user opted-in into new beta addons)
+    //
+    if (forceAddons === "false" && injectHostingIntegrations === "true") {
+      return new HTMLRewriter()
+        .on("head", new addPreloads())
+        .on("head", new addProjectVersionSlug(projectSlug, versionSlug))
+        .transform(originalResponse);
+    }
+  }
+
+  // Modify `_static/searchtools.js` to re-enable Sphinx's default search
+  if (
+    (contentType.includes("text/javascript") ||
+      contentType.includes("application/javascript")) &&
+    (injectHostingIntegrations === "true" || forceAddons === "true") &&
+    originalResponse.url.endsWith("_static/searchtools.js")
+  ) {
+    console.log("Modifying _static/searchtools.js");
+    return handleSearchToolsJSRequest(originalResponse);
+  }
+
+  // if none of the previous conditions are met,
+  // we return the response without modifying it
+  return originalResponse;
+}
+
+class removeElement {
+  element(element) {
+    console.log("Removing: " + element.tagName);
+    console.log("Attribute href: " + element.getAttribute("href"));
+    console.log("Attribute src: " + element.getAttribute("src"));
+    console.log("Attribute id: " + element.getAttribute("id"));
+    console.log("Attribute class: " + element.getAttribute("class"));
+    element.remove();
+  }
+}
+
+class addPreloads {
+  element(element) {
+    console.log("addPreloads");
+    element.append(addonsJs, { html: true });
+  }
+}
+
+class addProjectVersionSlug {
+  constructor(projectSlug, versionSlug) {
+    this.projectSlug = projectSlug;
+    this.versionSlug = versionSlug;
+  }
+
+  element(element) {
+    console.log(
+      `addProjectVersionSlug. projectSlug=${this.projectSlug} versionSlug=${this.versionSlug}`,
+    );
+    if (this.projectSlug && this.versionSlug) {
+      const metaProject = `<meta name="readthedocs-project-slug" content="${this.projectSlug}" />`;
+      const metaVersion = `<meta name="readthedocs-version-slug" content="${this.versionSlug}" />`;
+
+      element.append(metaProject, { html: true });
+      element.append(metaVersion, { html: true });
+    }
+  }
+}
+
+/*
+
+  Script to fix the old removal of the Sphinx search init.
+
+  Enabling addons breaks the default Sphinx search in old versions that are not possible to rebuilt.
+  This is because we solved the problem in the `readthedocs-sphinx-ext` extension,
+  but since those versions can't be rebuilt, the fix does not apply there.
+
+  To solve the problem in these old versions, we are using a CF worker to apply that fix on-the-fly
+  at serving time on those old versions.
+
+  The fix basically replaces a Read the Docs comment in file `_static/searchtools.js`,
+  introduced by `readthedocs-sphinx-ext` to _disable the initialization of Sphinx search_,
+  with the real JavaScript to initialize the search, as Sphinx does by default.
+  (in other words, it _reverts_ the manipulation done by `readthedocs-sphinx-ext`)
+
+*/
+
+const textToReplace = `/* Search initialization removed for Read the Docs */`;
+const textReplacement = `
+/* Search initialization manipulated by Read the Docs using Cloudflare Workers */
+/* See https://github.com/readthedocs/addons/issues/219 for more information */
+
+function initializeSearch() {
+  Search.init();
+}
+
+if (document.readyState !== "loading") {
+  initializeSearch();
+}
+else {
+  document.addEventListener("DOMContentLoaded", initializeSearch);
+}
+`;
+
+async function handleSearchToolsJSRequest(originalResponse) {
+  const content = await originalResponse.text();
+  const modifiedResponse = new Response(
+    content.replace(textToReplace, textReplacement),
+  );
+  return modifiedResponse;
+}

From 1cac3adf650c2b2587542816ace1a78d12c27b84 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Tue, 6 Feb 2024 13:45:56 +0100
Subject: [PATCH 03/10] Proxy JS files to Addons GitHub's repository

---
 dockerfiles/nginx/proxito.conf.template | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/dockerfiles/nginx/proxito.conf.template b/dockerfiles/nginx/proxito.conf.template
index f1d950c7710..be9d8cce4d9 100644
--- a/dockerfiles/nginx/proxito.conf.template
+++ b/dockerfiles/nginx/proxito.conf.template
@@ -1,7 +1,7 @@
 # Proxito
 server {
     listen 8080;
-    server_name nginx;
+    server_name proxito;
 
     # Docker Compose's "logging.driver: none" is not working anymore.
     # So, we are disabling the logs from NGINX directly.
@@ -20,6 +20,17 @@ server {
         rewrite ^ http://$project--$version.org.dev.readthedocs.build/page/$path;
     }
 
+    # Proxy the "readthedocs-addons.js" to be downloaded from GitHub
+    location /_/static/javascript/readthedocs-addons.js {
+        proxy_pass https://raw.githubusercontent.com/readthedocs/addons/$NGINX_ADDONS_GITHUB_TAG/dist/readthedocs-addons.js;
+        add_header Content-Type	"text/javascript; charset=utf-8" always;
+    }
+
+    location /_/static/javascript/readthedocs-addons.js.map {
+        proxy_pass https://raw.githubusercontent.com/readthedocs/addons/$NGINX_ADDONS_GITHUB_TAG/dist/readthedocs-addons.js.map;
+        add_header Content-Type	"text/javascript; charset=utf-8" always;
+    }
+
     # Proxito doc serving
     location / {
         proxy_pass http://proxito:8000;

From 44f0837caed831d31c44996a785799466d64b736 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Tue, 6 Feb 2024 13:46:29 +0100
Subject: [PATCH 04/10] Point common to wrangler branch

---
 common | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common b/common
index e598102991c..7065507e626 160000
--- a/common
+++ b/common
@@ -1 +1 @@
-Subproject commit e598102991c5a927024f06f187f51f7000e67023
+Subproject commit 7065507e62699d698bb6ca195009053ffc917380

From f3c3a3f074d330c0676b2faf614fdcaeebd8998e Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Thu, 8 Feb 2024 10:14:10 +0100
Subject: [PATCH 05/10] Addons + Proxito: return `X-RTD-Resolver-Filename` and
 inject via CF

Return `X-RTD-Resolver-Filename` from El Proxito and inject it as HTML `meta`
tag from Cloudflare Worker.

Required by https://github.com/readthedocs/addons/issues/211
---
 dockerfiles/force-readthedocs-addons.js   | 14 +++++++++-----
 dockerfiles/nginx/proxito.conf.template   |  2 ++
 readthedocs/proxito/middleware.py         | 22 ++++++++++++++++++++--
 readthedocs/proxito/tests/test_headers.py |  1 +
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/dockerfiles/force-readthedocs-addons.js b/dockerfiles/force-readthedocs-addons.js
index 161ff58c494..1961cd5d97e 100644
--- a/dockerfiles/force-readthedocs-addons.js
+++ b/dockerfiles/force-readthedocs-addons.js
@@ -70,6 +70,7 @@ async function handleRequest(request) {
   // get project/version slug from headers inject by El Proxito
   const projectSlug = originalResponse.headers.get("x-rtd-project") || "";
   const versionSlug = originalResponse.headers.get("x-rtd-version") || "";
+  const resolverFilename = originalResponse.headers.get("x-rtd-resolver-filename") || "";
 
   // check to decide whether or not inject the new beta addons:
   //
@@ -103,7 +104,7 @@ async function handleRequest(request) {
           // .on(readthedocsDataParse, new removeElement())
           // .on(readthedocsData, new removeElement())
           .on("head", new addPreloads())
-          .on("head", new addProjectVersionSlug(projectSlug, versionSlug))
+          .on("head", new addMetaTags(projectSlug, versionSlug, resolverFilename))
           .transform(originalResponse)
       );
     }
@@ -116,7 +117,7 @@ async function handleRequest(request) {
     if (forceAddons === "false" && injectHostingIntegrations === "true") {
       return new HTMLRewriter()
         .on("head", new addPreloads())
-        .on("head", new addProjectVersionSlug(projectSlug, versionSlug))
+        .on("head", new addMetaTags(projectSlug, versionSlug, resolverFilename))
         .transform(originalResponse);
     }
   }
@@ -155,22 +156,25 @@ class addPreloads {
   }
 }
 
-class addProjectVersionSlug {
-  constructor(projectSlug, versionSlug) {
+class addMetaTags {
+  constructor(projectSlug, versionSlug, resolverFilename) {
     this.projectSlug = projectSlug;
     this.versionSlug = versionSlug;
+    this.resolverFilename = resolverFilename;
   }
 
   element(element) {
     console.log(
-      `addProjectVersionSlug. projectSlug=${this.projectSlug} versionSlug=${this.versionSlug}`,
+      `addMetaTags. projectSlug=${this.projectSlug} versionSlug=${this.versionSlug} resolverFilename=${this.resolverFilename}`,
     );
     if (this.projectSlug && this.versionSlug) {
       const metaProject = `<meta name="readthedocs-project-slug" content="${this.projectSlug}" />`;
       const metaVersion = `<meta name="readthedocs-version-slug" content="${this.versionSlug}" />`;
+      const metaResolverFilename = `<meta name="readthedocs-resolver-filename" content="${this.resolverFilename}" />`;
 
       element.append(metaProject, { html: true });
       element.append(metaVersion, { html: true });
+      element.append(metaResolverFilename, { html: true });
     }
   }
 }
diff --git a/dockerfiles/nginx/proxito.conf.template b/dockerfiles/nginx/proxito.conf.template
index be9d8cce4d9..f8b2f80f5a4 100644
--- a/dockerfiles/nginx/proxito.conf.template
+++ b/dockerfiles/nginx/proxito.conf.template
@@ -75,6 +75,8 @@ server {
         add_header X-RTD-Project-Method $rtd_project_method always;
         set $rtd_redirect $upstream_http_x_rtd_redirect;
         add_header X-RTD-Redirect $rtd_redirect always;
+        set $rtd_resolver_filename $upstream_http_x_rtd_resolver_filename;
+        add_header X-RTD-Resolver-Filename $rtd_resolver_filename always;
         set $cdn_cache_control $upstream_http_cdn_cache_control;
         add_header CDN-Cache-Control $cdn_cache_control always;
         set $cache_tag $upstream_http_cache_tag;
diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py
index fbc200220f8..44f4494ddfb 100644
--- a/readthedocs/proxito/middleware.py
+++ b/readthedocs/proxito/middleware.py
@@ -23,9 +23,11 @@
 from readthedocs.core.unresolver import (
     InvalidCustomDomainError,
     InvalidExternalDomainError,
+    InvalidPathForVersionedProjectError,
     InvalidSubdomainError,
     InvalidXRTDSlugHeaderError,
     SuspiciousHostnameError,
+    unresolve,
     unresolver,
 )
 from readthedocs.core.utils import get_cache_tag
@@ -187,17 +189,19 @@ def add_cache_headers(self, request, response):
         else:
             cache_response(response, force=False)
 
-    def _set_request_attributes(self, request, unresolved_domain):
+    def _set_request_attributes(self, request, unresolved_domain, unresolved_url):
         """
         Set attributes in the request from the unresolved domain.
 
         - Set ``request.unresolved_domain`` to the unresolved domain.
         """
         request.unresolved_domain = unresolved_domain
+        request.unresolved_url = unresolved_url
 
     def process_request(self, request):  # noqa
         # Initialize our custom request attributes.
         request.unresolved_domain = None
+        request.unresolved_url = None
 
         skip = any(request.path.startswith(reverse(view)) for view in self.skip_views)
         if skip:
@@ -229,7 +233,12 @@ def process_request(self, request):  # noqa
         except InvalidXRTDSlugHeaderError as exc:
             raise SuspiciousOperation("Invalid X-RTD-Slug header.") from exc
 
-        self._set_request_attributes(request, unresolved_domain)
+        try:
+            unresolved_url = unresolve(request.build_absolute_uri())
+        except InvalidPathForVersionedProjectError:
+            unresolved_url = None
+
+        self._set_request_attributes(request, unresolved_domain, unresolved_url)
 
         response = self._get_https_redirect(request)
         if response:
@@ -367,11 +376,20 @@ def _get_https_redirect(self, request):
 
         return None
 
+    def add_resolver_headers(self, request, response):
+        # TODO: find a better way to re-use the unresolved URL so we don't
+        # query the db multiple times on the same request.
+        # https://github.com/readthedocs/readthedocs.org/issues/10456
+        if request.unresolved_url is not None:
+            # TODO: add more ``X-RTD-Resolver-*`` headers
+            response["X-RTD-Resolver-Filename"] = request.unresolved_url.filename
+
     def process_response(self, request, response):  # noqa
         self.add_proxito_headers(request, response)
         self.add_cache_headers(request, response)
         self.add_hsts_headers(request, response)
         self.add_user_headers(request, response)
         self.add_hosting_integrations_headers(request, response)
+        self.add_resolver_headers(request, response)
         self.add_cors_headers(request, response)
         return response
diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py
index 1b2ce6726b4..84582a0d066 100644
--- a/readthedocs/proxito/tests/test_headers.py
+++ b/readthedocs/proxito/tests/test_headers.py
@@ -51,6 +51,7 @@ def test_serve_headers(self):
         self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
         self.assertEqual(r["X-RTD-Version"], "latest")
         self.assertEqual(r["X-RTD-version-Method"], "path")
+        self.assertEqual(r["X-RTD-Resolver-Filename"], "/index.html")
         self.assertEqual(
             r["X-RTD-Path"], "/proxito/media/html/project/latest/index.html"
         )

From 9b46d698e5470d8d704c5d2eb5276d57561275e3 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Tue, 13 Feb 2024 12:40:06 +0100
Subject: [PATCH 06/10] Feedback from review

---
 readthedocs/proxito/middleware.py         | 20 ++---
 readthedocs/proxito/tests/test_headers.py |  2 +-
 readthedocs/proxito/views/serve.py        | 94 +++++++++++++----------
 3 files changed, 61 insertions(+), 55 deletions(-)

diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py
index 44f4494ddfb..e2489a6a5cb 100644
--- a/readthedocs/proxito/middleware.py
+++ b/readthedocs/proxito/middleware.py
@@ -18,16 +18,15 @@
 from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils.deprecation import MiddlewareMixin
+from django.utils.html import escape
 
 from readthedocs.builds.models import Version
 from readthedocs.core.unresolver import (
     InvalidCustomDomainError,
     InvalidExternalDomainError,
-    InvalidPathForVersionedProjectError,
     InvalidSubdomainError,
     InvalidXRTDSlugHeaderError,
     SuspiciousHostnameError,
-    unresolve,
     unresolver,
 )
 from readthedocs.core.utils import get_cache_tag
@@ -189,19 +188,17 @@ def add_cache_headers(self, request, response):
         else:
             cache_response(response, force=False)
 
-    def _set_request_attributes(self, request, unresolved_domain, unresolved_url):
+    def _set_request_attributes(self, request, unresolved_domain):
         """
         Set attributes in the request from the unresolved domain.
 
         - Set ``request.unresolved_domain`` to the unresolved domain.
         """
         request.unresolved_domain = unresolved_domain
-        request.unresolved_url = unresolved_url
 
     def process_request(self, request):  # noqa
         # Initialize our custom request attributes.
         request.unresolved_domain = None
-        request.unresolved_url = None
 
         skip = any(request.path.startswith(reverse(view)) for view in self.skip_views)
         if skip:
@@ -233,12 +230,7 @@ def process_request(self, request):  # noqa
         except InvalidXRTDSlugHeaderError as exc:
             raise SuspiciousOperation("Invalid X-RTD-Slug header.") from exc
 
-        try:
-            unresolved_url = unresolve(request.build_absolute_uri())
-        except InvalidPathForVersionedProjectError:
-            unresolved_url = None
-
-        self._set_request_attributes(request, unresolved_domain, unresolved_url)
+        self._set_request_attributes(request, unresolved_domain)
 
         response = self._get_https_redirect(request)
         if response:
@@ -380,9 +372,11 @@ def add_resolver_headers(self, request, response):
         # TODO: find a better way to re-use the unresolved URL so we don't
         # query the db multiple times on the same request.
         # https://github.com/readthedocs/readthedocs.org/issues/10456
-        if request.unresolved_url is not None:
+        if hasattr(request, "unresolved_url") and request.unresolved_url is not None:
             # TODO: add more ``X-RTD-Resolver-*`` headers
-            response["X-RTD-Resolver-Filename"] = request.unresolved_url.filename
+            response["X-RTD-Resolver-Filename"] = escape(
+                request.unresolved_url.filename
+            )
 
     def process_response(self, request, response):  # noqa
         self.add_proxito_headers(request, response)
diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py
index 84582a0d066..ed778a2f251 100644
--- a/readthedocs/proxito/tests/test_headers.py
+++ b/readthedocs/proxito/tests/test_headers.py
@@ -51,7 +51,7 @@ def test_serve_headers(self):
         self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
         self.assertEqual(r["X-RTD-Version"], "latest")
         self.assertEqual(r["X-RTD-version-Method"], "path")
-        self.assertEqual(r["X-RTD-Resolver-Filename"], "/index.html")
+        self.assertEqual(r["X-RTD-Resolver-Filename"], "/")
         self.assertEqual(
             r["X-RTD-Path"], "/proxito/media/html/project/latest/index.html"
         )
diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py
index 5bf63950bc5..3d2d4a2c58f 100644
--- a/readthedocs/proxito/views/serve.py
+++ b/readthedocs/proxito/views/serve.py
@@ -265,6 +265,10 @@ def serve_path(self, request, path):
         version = unresolved.version
         filename = unresolved.filename
 
+        # Inject the UnresolvedURL into the HttpRequest so we can access from the middleware.
+        # We could resolve it again from the middleware, but we would duplicating DB queries.
+        request.unresolved_url = unresolved
+
         # Check if the old language code format was used, and redirect to the new one.
         # NOTE: we may have some false positives here, for example for an URL like:
         # /pt-br/latest/pt_BR/index.html, but our protection for infinite redirects
@@ -391,7 +395,7 @@ def get(self, request, proxito_path):
         the Docs default page (Maze Found) is rendered by Django and served.
         """
         log.bind(proxito_path=proxito_path)
-        log.debug('Executing 404 handler.')
+        log.debug("Executing 404 handler.")
         unresolved_domain = request.unresolved_domain
         # We force all storage calls to use the external versions storage,
         # since we are serving an external version.
@@ -708,11 +712,12 @@ def get(self, request):
         # Verify if the project is marked as spam and return a custom robots.txt
         if "readthedocsext.spamfighting" in settings.INSTALLED_APPS:
             from readthedocsext.spamfighting.utils import is_robotstxt_denied  # noqa
+
             if is_robotstxt_denied(project):
                 return render(
                     request,
-                    'robots.spam.txt',
-                    content_type='text/plain',
+                    "robots.spam.txt",
+                    content_type="text/plain",
                 )
 
         # Use the ``robots.txt`` file from the default version configured
@@ -747,33 +752,30 @@ def get(self, request):
                 filename="robots.txt",
                 check_if_exists=True,
             )
-            log.info('Serving custom robots.txt file.')
+            log.info("Serving custom robots.txt file.")
             return response
         except StorageFileNotFound:
             pass
 
         # Serve default robots.txt
-        sitemap_url = '{scheme}://{domain}/sitemap.xml'.format(
-            scheme='https',
+        sitemap_url = "{scheme}://{domain}/sitemap.xml".format(
+            scheme="https",
             domain=project.subdomain(),
         )
         context = {
-            'sitemap_url': sitemap_url,
-            'hidden_paths': self._get_hidden_paths(project),
+            "sitemap_url": sitemap_url,
+            "hidden_paths": self._get_hidden_paths(project),
         }
         return render(
             request,
-            'robots.txt',
+            "robots.txt",
             context,
-            content_type='text/plain',
+            content_type="text/plain",
         )
 
     def _get_hidden_paths(self, project):
         """Get the absolute paths of the public hidden versions of `project`."""
-        hidden_versions = (
-            Version.internal.public(project=project)
-            .filter(hidden=True)
-        )
+        hidden_versions = Version.internal.public(project=project).filter(hidden=True)
         resolver = Resolver()
         hidden_paths = [
             resolver.resolve_path(project, version_slug=version.slug)
@@ -846,8 +848,8 @@ def hreflang_formatter(lang):
             Use hyphen instead of underscore in language and country value.
             ref: https://en.wikipedia.org/wiki/Hreflang#Common_Mistakes
             """
-            if '_' in lang:
-                return lang.replace('_', '-')
+            if "_" in lang:
+                return lang.replace("_", "-")
             return lang
 
         def changefreqs_generator():
@@ -861,8 +863,8 @@ def changefreqs_generator():
             aggressive. If the tag is removed and a branch is created with the same
             name, we will want bots to revisit this.
             """
-            changefreqs = ['weekly', 'daily']
-            yield from itertools.chain(changefreqs, itertools.repeat('monthly'))
+            changefreqs = ["weekly", "daily"]
+            yield from itertools.chain(changefreqs, itertools.repeat("monthly"))
 
         project = request.unresolved_domain.project
         public_versions = Version.internal.public(
@@ -879,28 +881,34 @@ def changefreqs_generator():
         # We want stable with priority=1 and changefreq='weekly' and
         # latest with priority=0.9 and changefreq='daily'
         # More details on this: https://github.com/rtfd/readthedocs.org/issues/5447
-        if (len(sorted_versions) >= 2 and sorted_versions[0].slug == LATEST and
-                sorted_versions[1].slug == STABLE):
-            sorted_versions[0], sorted_versions[1] = sorted_versions[1], sorted_versions[0]
+        if (
+            len(sorted_versions) >= 2
+            and sorted_versions[0].slug == LATEST
+            and sorted_versions[1].slug == STABLE
+        ):
+            sorted_versions[0], sorted_versions[1] = (
+                sorted_versions[1],
+                sorted_versions[0],
+            )
 
         versions = []
         for version, priority, changefreq in zip(
-                sorted_versions,
-                priorities_generator(),
-                changefreqs_generator(),
+            sorted_versions,
+            priorities_generator(),
+            changefreqs_generator(),
         ):
             element = {
-                'loc': version.get_subdomain_url(),
-                'priority': priority,
-                'changefreq': changefreq,
-                'languages': [],
+                "loc": version.get_subdomain_url(),
+                "priority": priority,
+                "changefreq": changefreq,
+                "languages": [],
             }
 
             # Version can be enabled, but not ``built`` yet. We want to show the
             # link without a ``lastmod`` attribute
-            last_build = version.builds.order_by('-date').first()
+            last_build = version.builds.order_by("-date").first()
             if last_build:
-                element['lastmod'] = last_build.date.isoformat()
+                element["lastmod"] = last_build.date.isoformat()
 
             resolver = Resolver()
             if project.translations.exists():
@@ -915,27 +923,31 @@ def changefreqs_generator():
                             project=translation,
                             version=translated_version,
                         )
-                        element['languages'].append({
-                            'hreflang': hreflang_formatter(translation.language),
-                            'href': href,
-                        })
+                        element["languages"].append(
+                            {
+                                "hreflang": hreflang_formatter(translation.language),
+                                "href": href,
+                            }
+                        )
 
                 # Add itself also as protocol requires
-                element['languages'].append({
-                    'hreflang': project.language,
-                    'href': element['loc'],
-                })
+                element["languages"].append(
+                    {
+                        "hreflang": project.language,
+                        "href": element["loc"],
+                    }
+                )
 
             versions.append(element)
 
         context = {
-            'versions': versions,
+            "versions": versions,
         }
         return render(
             request,
-            'sitemap.xml',
+            "sitemap.xml",
             context,
-            content_type='application/xml',
+            content_type="application/xml",
         )
 
     def _get_project(self):

From 01f96f109b793de708bfd1e86977cc54d1f17f17 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Tue, 13 Feb 2024 12:54:37 +0100
Subject: [PATCH 07/10] Validate the header value before injecting it

---
 readthedocs/proxito/middleware.py             | 24 ++++++++++++++-----
 .../proxito/tests/test_old_redirects.py       |  1 +
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py
index e2489a6a5cb..8bba1f723fd 100644
--- a/readthedocs/proxito/middleware.py
+++ b/readthedocs/proxito/middleware.py
@@ -15,6 +15,7 @@
 )
 from django.conf import settings
 from django.core.exceptions import SuspiciousOperation
+from django.http.response import BadHeaderError, ResponseHeaders
 from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils.deprecation import MiddlewareMixin
@@ -369,14 +370,25 @@ def _get_https_redirect(self, request):
         return None
 
     def add_resolver_headers(self, request, response):
-        # TODO: find a better way to re-use the unresolved URL so we don't
-        # query the db multiple times on the same request.
-        # https://github.com/readthedocs/readthedocs.org/issues/10456
         if hasattr(request, "unresolved_url") and request.unresolved_url is not None:
             # TODO: add more ``X-RTD-Resolver-*`` headers
-            response["X-RTD-Resolver-Filename"] = escape(
-                request.unresolved_url.filename
-            )
+            header_value = escape(request.unresolved_url.filename)
+            try:
+                # Use Django internals to validate the header's value before injecting it.
+                ResponseHeaders({})._convert_to_charset(
+                    header_value,
+                    "latin-1",
+                    mime_encode=True,
+                )
+
+                response["X-RTD-Resolver-Filename"] = header_value
+            except BadHeaderError:
+                # Skip adding the header because it fails validation
+                log.info(
+                    "Skip adding X-RTD-Resolver-Filename header due to invalid value.",
+                    filename=request.unresolved_url.filename,
+                    value=header_value,
+                )
 
     def process_response(self, request, response):  # noqa
         self.add_proxito_headers(request, response)
diff --git a/readthedocs/proxito/tests/test_old_redirects.py b/readthedocs/proxito/tests/test_old_redirects.py
index e2a388ac7c6..a1604668e82 100644
--- a/readthedocs/proxito/tests/test_old_redirects.py
+++ b/readthedocs/proxito/tests/test_old_redirects.py
@@ -1352,6 +1352,7 @@ def test_redirect_exact_with_wildcard_crossdomain(self):
             r = self.client.get(url, headers={"host": "project.dev.readthedocs.io"})
             self.assertEqual(r.status_code, 302, url)
             self.assertEqual(r["Location"], expected_location, url)
+            self.assertNotIn("X-RTD-Resolver-Filename", r.headers)
 
     def test_redirect_html_to_clean_url_crossdomain(self):
         """

From ae242f5330be4d97e3544a8f0cc0c1c05ebbfdb6 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Wed, 14 Feb 2024 12:37:53 +0100
Subject: [PATCH 08/10] Initialize `request.unresolved_url` as `None`

---
 readthedocs/proxito/middleware.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/readthedocs/proxito/middleware.py b/readthedocs/proxito/middleware.py
index 8bba1f723fd..b6807967a74 100644
--- a/readthedocs/proxito/middleware.py
+++ b/readthedocs/proxito/middleware.py
@@ -200,6 +200,7 @@ def _set_request_attributes(self, request, unresolved_domain):
     def process_request(self, request):  # noqa
         # Initialize our custom request attributes.
         request.unresolved_domain = None
+        request.unresolved_url = None
 
         skip = any(request.path.startswith(reverse(view)) for view in self.skip_views)
         if skip:
@@ -370,7 +371,7 @@ def _get_https_redirect(self, request):
         return None
 
     def add_resolver_headers(self, request, response):
-        if hasattr(request, "unresolved_url") and request.unresolved_url is not None:
+        if request.unresolved_url is not None:
             # TODO: add more ``X-RTD-Resolver-*`` headers
             header_value = escape(request.unresolved_url.filename)
             try:

From 3e9d04ae8be934ed288bfcc9b306b22ea6bc26b2 Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Wed, 14 Feb 2024 12:44:28 +0100
Subject: [PATCH 09/10] Add test for longer filename

---
 readthedocs/proxito/tests/test_headers.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/readthedocs/proxito/tests/test_headers.py b/readthedocs/proxito/tests/test_headers.py
index ed778a2f251..f99a6a3dc45 100644
--- a/readthedocs/proxito/tests/test_headers.py
+++ b/readthedocs/proxito/tests/test_headers.py
@@ -56,6 +56,25 @@ def test_serve_headers(self):
             r["X-RTD-Path"], "/proxito/media/html/project/latest/index.html"
         )
 
+    def test_serve_headers_with_path(self):
+        r = self.client.get(
+            "/en/latest/guides/jupyter/gallery.html",
+            secure=True,
+            headers={"host": "project.dev.readthedocs.io"},
+        )
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r["Cache-Tag"], "project,project:latest")
+        self.assertEqual(r["X-RTD-Domain"], "project.dev.readthedocs.io")
+        self.assertEqual(r["X-RTD-Project"], "project")
+        self.assertEqual(r["X-RTD-Project-Method"], "public_domain")
+        self.assertEqual(r["X-RTD-Version"], "latest")
+        self.assertEqual(r["X-RTD-version-Method"], "path")
+        self.assertEqual(r["X-RTD-Resolver-Filename"], "/guides/jupyter/gallery.html")
+        self.assertEqual(
+            r["X-RTD-Path"],
+            "/proxito/media/html/project/latest/guides/jupyter/gallery.html",
+        )
+
     def test_subproject_serve_headers(self):
         r = self.client.get(
             "/projects/subproject/en/latest/",

From 7085007fb038888b50b7e30ea180df51358937da Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Wed, 14 Feb 2024 12:53:27 +0100
Subject: [PATCH 10/10] Inject the `unresolved_url` on 404 pages

---
 readthedocs/proxito/views/serve.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py
index 3d2d4a2c58f..4f44d51fa07 100644
--- a/readthedocs/proxito/views/serve.py
+++ b/readthedocs/proxito/views/serve.py
@@ -424,6 +424,11 @@ def get(self, request, proxito_path):
                 path=proxito_path,
                 append_indexhtml=False,
             )
+
+            # Inject the UnresolvedURL into the HttpRequest so we can access from the middleware.
+            # We could resolve it again from the middleware, but we would duplicating DB queries.
+            request.unresolved_url = unresolved
+
             project = unresolved.project
             version = unresolved.version
             filename = unresolved.filename