Add support for Opaque redirects (#1318)

* Add support for Opaque redirects

* Fix redirect option

* Determine redirect based on `status` property

Since we were relying on browser spec which supports a
concept of opaque-redirect, the Cloufdlare Workers on the other
hand don't suppport certain properties which make this possible.

They, instead make the actual status code available in redirect
manual mode which is now being used to determine whether the
request is redirect.

* change a test def

* Reduce the brittleness of test

* Run prettier of cloudflare-optimizer-scripts

* Rewrite Location Header for a redirect back to origin in proxy mode

* Handle Location rewrites for non-get requests

* styling fix for maybeRewriteRedirectHeader

Co-authored-by: Jake Fried <[email protected]>

* remove non-useful event.respondWith assertion

Co-authored-by: Jake Fried <[email protected]>

* use Headers init constructor

Co-authored-by: Jake Fried <[email protected]>

* clarity in test title

Co-authored-by: Jake Fried <[email protected]>

* reuse isRedirect and isReverseProxy

* Restructure rewrite header logic for non-Get metods

Co-authored-by: Jake Fried <[email protected]>

Author: abdulhannanali

packages/cloudflare-optimizer-scripts/src/index.js

@@ -66,14 +66,22 @@ async function handleRequest(event, config) {
 
   let request = event.request;
   const url = new URL(request.url);
+
   if (isReverseProxy(config)) {
     url.hostname = config.proxy.origin;
   }
+
   request = new Request(url.toString(), request);
 
   // Immediately return if not GET.
   if (request.method !== 'GET') {
-    return fetch(request);
+    if (!isReverseProxy(config)) {
+      return fetch(request, {redirect: 'manual'});
+    }
+
+    const response = await fetch(request, {redirect: 'manual'});
+
+    return isRedirect(response) ? rewriteRedirectHeader(response, config) : response;
   }
 
   if (config.enableKVCache) {
@@ -85,7 +93,17 @@ async function handleRequest(event, config) {
     }
   }
 
-  const response = await fetch(request);
+  const response = await fetch(request, {redirect: 'manual'});
+
+  // Redirect based statuses should be returned
+  if (isRedirect(response)) {
+    if (isReverseProxy(config)) {
+      return rewriteRedirectHeader(response, config);
+    }
+
+    return response;
+  }
+
   const clonedResponse = response.clone();
   const {headers, status, statusText} = response;
 
@@ -259,6 +277,44 @@ function resetOptimizerForTesting() {
   ampOptimizer = null;
 }
 
+/**
+ * @description Determine whether {Response} is Redirect style
+ * @param {Response} response
+ * @returns {boolean}
+ */
+function isRedirect(response) {
+  return response.status >= 300 && response.status <= 399;
+}
+
+/**
+ * @param {!Response} response
+ * @param {!ConfigDef} config
+ * @description Rewrites the location header to reflect the worker's hostname
+ */
+function rewriteRedirectHeader(response, config) {
+  const locationHeader = response.headers.get('location');
+
+  if (!locationHeader) {
+    return response;
+  }
+
+  const parsedLocation = new URL(locationHeader);
+
+  if (config.proxy.origin === parsedLocation.hostname) {
+    parsedLocation.hostname = config.proxy.worker;
+    const newHeaders = new Headers(response.headers);
+    newHeaders.set('location', parsedLocation.toString());
+
+    return new Response(response.body, {
+      headers: newHeaders,
+      status: response.status,
+      statusText: response.statusText,
+    });
+  }
+
+  return response;
+}
+
 module.exports = {
   getOptimizer,
   handleEvent,

packages/cloudflare-optimizer-scripts/test/builtins.js

@@ -45,4 +45,6 @@ class HTMLRewriter {
   }
 }
 
-module.exports = {Response, HTMLRewriter, Request};
+class Headers extends Map {}
+
+module.exports = {Response, HTMLRewriter, Request, Headers};

packages/cloudflare-optimizer-scripts/test/index.test.js

@@ -30,7 +30,7 @@ const {
   validateConfiguration,
   resetOptimizerForTesting,
 } = require('../src/index');
-const {Request, Response, HTMLRewriter} = require('./builtins');
+const {Request, Response, HTMLRewriter, Headers} = require('./builtins');
 
 beforeEach(() => {
   global.fetch = jest.fn();
@@ -42,6 +42,7 @@ beforeEach(() => {
   global.Request = Request;
   global.Response = Response;
   global.HTMLRewriter = HTMLRewriter;
+  global.Headers = Headers;
 });
 
 describe('handleEvent', () => {
@@ -70,11 +71,31 @@ describe('handleEvent', () => {
     expect(await event.respondWith.mock.calls[0][0]).toBe(incomingResponse);
   });
 
+  it('should rewrite Location header for Redirect in response to Non-Get Requests (Proxy Mode)', async () => {
+    const config = {proxy: {origin: 'test-origin.com', worker: 'test.com'}};
+
+    const originResponse = new Response(undefined, {
+      status: 301,
+      statusText: '',
+      headers: new Headers([['location', 'https://test-origin.com/abc']]),
+    });
+
+    global.fetch.mockImplementation(() => Promise.resolve(originResponse));
+
+    const request = {url: 'http://test.com', method: 'POST'};
+    const event = {request, passThroughOnException: jest.fn(), respondWith: jest.fn()};
+    handleEvent(event, {...defaultConfig, ...config});
+
+    const response = await event.respondWith.mock.calls[0][0];
+    expect(response.status).toBe(301);
+    expect(response.headers.get('location')).toBe('https://test.com/abc');
+  });
+
   it('Should proxy through optimizer failures', async () => {
     const input = `<html amp><body></body></html>`;
     const incomingResponse = getResponse(input);
     global.fetch.mockReturnValue(incomingResponse);
-    AmpOptimizer.transformHtmlSpy.mockReturnValue(Promise.reject('Fail.'));
+    AmpOptimizer.transformHtmlSpy.mockImplementation(() => Promise.reject('Fail.'));
 
     const output = await getOutput('http://text.com');
     expect(output).toBe(input);
@@ -105,11 +126,58 @@ describe('handleEvent', () => {
     expect(output).toBe(`transformed-${input}`);
   });
 
+  it('should passthrough opaque redirects unmodified (non-proxy mode)', async () => {
+    const mockedResponse = new Response('', {
+      status: 302,
+      statusText: '',
+    });
+
+    const event = {
+      request: {method: 'GET', url: 'https://example.com/'},
+      respondWith: jest.fn(),
+      passThroughOnException: jest.fn(),
+    };
+
+    global.fetch.mockImplementation(() => Promise.resolve(mockedResponse));
+
+    handleEvent(event, defaultConfig);
+    expect(await event.respondWith.mock.calls[0][0]).toBe(mockedResponse);
+  });
+
+  it('should rewrite location header for redirects to origin in proxy mode', async () => {
+    const config = {proxy: {worker: 'test.com', origin: 'test-origin.com'}};
+
+    const mockedResponse = new Response('', {
+      status: 302,
+      statusText: '',
+      headers: new Headers([['location', 'https://test-origin.com/abc']]),
+    });
+
+    const event = {
+      request: {method: 'GET', url: 'https://test.com/'},
+      respondWith: jest.fn(),
+      passThroughOnException: jest.fn(),
+    };
+
+    global.fetch.mockImplementation(() => Promise.resolve(mockedResponse));
+
+    handleEvent(event, {...defaultConfig, ...config});
+
+    const response = await event.respondWith.mock.calls[0][0];
+
+    expect(response.status).toBe(mockedResponse.status);
+    expect(response.headers.get('location')).toBe('https://test.com/abc');
+    expect(event.respondWith).toHaveBeenCalledTimes(1);
+  });
+
   it('Should passthrough request to origin in request interceptor mode', async () => {
     const input = `<html amp><body></body></html>`;
     global.fetch.mockReturnValue(getResponse(input));
     await getOutput('http://test.com');
-    expect(global.fetch).toBeCalledWith({url: 'http://test.com/', method: 'GET'});
+    expect(global.fetch).toBeCalledWith(
+      {url: 'http://test.com/', method: 'GET'},
+      {redirect: 'manual'}
+    );
   });
 
   it('Should modify request url for reverse-proxy', async () => {
@@ -118,7 +186,10 @@ describe('handleEvent', () => {
     global.fetch.mockReturnValue(getResponse(input));
 
     await getOutput('http://test.com', config);
-    expect(global.fetch).toBeCalledWith({url: 'http://test-origin.com/', method: 'GET'});
+    expect(global.fetch).toBeCalledWith(
+      {url: 'http://test-origin.com/', method: 'GET'},
+      {redirect: 'manual'}
+    );
   });
 
   it('should call enable passThroughOnException', async () => {
@@ -180,7 +251,7 @@ describe('getAmpOptimizer', () => {
 
 function getResponse(html, {contentType} = {contentType: 'text/html'}) {
   return new Response(html, {
-    headers: {get: () => contentType},
+    headers: new Headers([['content-type', contentType]]),
     status: 200,
     statusText: '200',
   });