update todos

lib/client/unblocker-client.js

@@ -4,6 +4,13 @@
   // todo:
   // - postMessage
   // - open
+  // - DOM Mutation Observer
+  //   - href
+  //   - src
+  //   - srcset
+  //   - style (this could get tricky...)
+  //   - poster (on <video> elements)
+  //   - perhaps some/all of this could be shared by the server-side url-rewriter
   // - split each part into separate files (?)
   // - wrap other JS and provide proxies to fix writes to window.location and document.cookie
   //   - will require updating contentTypes.html.includes(data.contentType) to include js
@@ -87,6 +94,8 @@
     };
   }
 
+  // this prevents an initial request to the wrong (unproxied) URL
+  // it also is important for <img> and <audio> elements that are only created in memory, and never added to the DOM
   function initCreateElement(config, window) {
     if (!window.document || !window.document.createElement) return;
     var _createElement = window.document.createElement;
@@ -103,15 +112,15 @@
         },
         configurable: true,
       });
+      // todo: let a DOM mutation observer handle href attributes when they're added to the document
       Object.defineProperty(element, "href", {
         set: function (href) {
           delete element.href; // remove this setter so we don't get stuck in an infinite loop
           element.href = fixUrl(href, config, location);
         },
         configurable: true,
       });
-      // todo: consider restoring the setter in case the client js changes the value later...
-      // todo: support srcset
+      // todo: consider restoring the setter in case the client js changes the value later (does that happen?)
       return element;
     };
   }

lib/unblocker.js

@@ -89,7 +89,7 @@ function Unblocker(config) {
     }
   }
 
-  // todo: check if config.debug is enabled first
+  // the middleware debugger logs details before/after each piece of middleware
   if (middlewareDebugger.enabled) {
     config.requestMiddleware = middlewareDebugger.debugMiddleware(
       config.requestMiddleware,
@@ -112,7 +112,7 @@ function Unblocker(config) {
    * proxy's filter, this checks the referrer to determine what the path should be, and then issues a
    * 307 redirect to a proxied url at that path
    *
-   * todo: handle querystring and post data
+   * 307 redirects cause the client to re-use the original method and body at the new location
    */
   function recoverTargetUrl(request) {
     if (request.url.indexOf(config.prefix) === 0) {
@@ -146,7 +146,6 @@ function Unblocker(config) {
       var target_url = real_uri.protocol + "//" + real_uri.host + request.url;
       debug("recovering broken link to %s", request.url);
       // now, take the requested pat on the previous known host and send the user on their way
-      // todo: make sure req.url includes the querystring
       return target_url;
     }
 
@@ -192,7 +191,7 @@ function Unblocker(config) {
         })
       );
     } catch (ex) {
-      // the headers were already sent - we can't redirect them
+      // Most likely because the headers were already sent
       console.error("Failed to send redirect", ex);
     }
     response.end();

test/unblocker_spec.js

@@ -79,18 +79,18 @@ test("should redirect root-relative urls when the correct target can be determin
         });
       }
       hyperquest(
-        servers.homeUrl + "bar",
+        servers.homeUrl + "bar?query_param=new",
         {
           headers: {
-            referer: servers.proxiedUrl + "foo",
+            referer: servers.proxiedUrl + "foo?query_param=old",
           },
         },
         function (err, res) {
           t.notOk(err);
           t.equal(res.statusCode, 307, "http status code");
           t.equal(
             res.headers.location,
-            servers.proxiedUrl + "bar",
+            servers.proxiedUrl + "bar?query_param=new",
             "redirect location"
           );
           cleanup();