fix: don't respond when headers already sent

fixes #2276
adobe · Nov 17, 2023 · 2df03e0 · 2df03e0
1 parent 23e3f40
commit 2df03e0
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 35 deletions.
diff --git a/src/server/HelixServer.js b/src/server/HelixServer.js
@@ -42,6 +42,7 @@ export class HelixServer extends BaseServer {
   async handleProxyModeRequest(req, res) {
     const sendFile = promisify(res.sendFile).bind(res);
     const ctx = new RequestContext(req, this._project);
+    const { id } = ctx;
     const { log } = this;
     const proxyUrl = new URL(this._project.proxyUrl);
 
@@ -56,10 +57,19 @@ export class HelixServer extends BaseServer {
     if (liveReload) {
       liveReload.startRequest(ctx.requestId, ctx.path);
     }
+    const pfx = log.level === 'silly' ? `[${id}] ` : '';
+
+    if (log.level === 'silly') {
+      log.trace(`${pfx}>>>--------------------------`);
+      log.trace(`${pfx}> ${req.method} ${req.url}`);
+      Object.entries(req.headers).forEach(([name, value]) => {
+        log.trace(`${pfx}> ${name}: ${value}`);
+      });
+      log.trace(`[${id}]`);
+    }
 
     // try to serve static
     try {
-      log.debug('trying to serve local file', filePath);
       await sendFile(filePath, {
         dotfiles: 'allow',
         headers: {
@@ -69,9 +79,13 @@ export class HelixServer extends BaseServer {
       if (liveReload) {
         liveReload.registerFile(ctx.requestId, filePath);
       }
+      log.debug(`${pfx}served local file ${filePath}`);
       return;
     } catch (e) {
-      log.debug(`Error while delivering resource ${ctx.path} - ${e.stack || e}`);
+      log.debug(`${pfx}unable to deliver local file ${ctx.path} - ${e.stack || e}`);
+      if (res.headersSent) {
+        return;
+      }
     } finally {
       if (liveReload) {
         liveReload.endRequest(ctx.requestId);
@@ -92,7 +106,7 @@ export class HelixServer extends BaseServer {
         file404html: this._project.file404html,
       });
     } catch (err) {
-      log.error(`Failed to proxy AEM request ${ctx.path}: ${err.message}`);
+      log.error(`${pfx}failed to proxy AEM request ${ctx.path}: ${err.message}`);
       res.status(502).send(`Failed to proxy AEM request: ${err.message}`);
     }
 

diff --git a/src/server/RequestContext.js b/src/server/RequestContext.js
@@ -12,6 +12,8 @@
 import { parse } from 'url';
 import utils from './utils.js';
 
+let id = 0;
+
 /**
  * Context that is used during request handling.
  *
@@ -26,6 +28,8 @@ export default class RequestContext {
     const { url } = req;
     this._cfg = cfg || {};
     this._url = url;
+    // eslint-disable-next-line no-plusplus
+    this._id = id++;
     const purl = parse(url);
     this._path = purl.pathname || '/';
     this._queryString = purl.search || '';
@@ -74,6 +78,14 @@ export default class RequestContext {
     this._resourcePath = relPath;
   }
 
+  /**
+   * returns the request id / counter
+   * @return {*|number}
+   */
+  get id() {
+    return this._id;
+  }
+
   /**
    * the original request url
    */

diff --git a/src/server/utils.js b/src/server/utils.js
@@ -215,7 +215,8 @@ window.LiveReloadOptions = {
    * @return {Promise} A promise that resolves when the stream is done.
    */
   async proxyRequest(ctx, url, req, res, opts = {}) {
-    ctx.log.debug(`Proxy ${req.method} request to ${url}`);
+    const { id } = ctx;
+    ctx.log.debug(`[${id}] Proxy ${req.method} request to ${url}`);
 
     if (opts.cacheDirectory) {
       const cached = await utils.getFromCache(url, opts.cacheDirectory, ctx.log);
@@ -263,19 +264,20 @@ window.LiveReloadOptions = {
     });
     const contentType = ret.headers.get('content-type') || 'text/plain';
     const level = utils.status2level(ret.status, true);
-    ctx.log[level](`Proxy ${req.method} request to ${url}: ${ret.status} (${contentType})`);
+    ctx.log[level](`[${id}] Proxy ${req.method} request to ${url}: ${ret.status} (${contentType})`);
 
     // because fetch decodes the response, we need to reset content encoding and length
     const respHeaders = Object.fromEntries(ret.headers.entries());
     delete respHeaders['content-encoding'];
     delete respHeaders['content-length'];
     delete respHeaders['x-frame-options'];
     delete respHeaders['content-security-policy'];
+    delete respHeaders.connection;
     respHeaders['access-control-allow-origin'] = '*';
     respHeaders.via = `${ret.httpVersion ?? '1.0'} ${new URL(url).hostname}`;
 
     if (ret.status === 404 && contentType.indexOf('text/html') === 0 && opts.file404html) {
-      ctx.log.debug('serve local 404.html', opts.file404html);
+      ctx.log.debug(`[${id}] serve local 404.html ${opts.file404html}`);
       let textBody = await fs.readFile(opts.file404html, 'utf-8');
       if (opts.injectLiveReload) {
         textBody = utils.injectLiveReloadScript(textBody, ctx.config.server);
@@ -292,34 +294,19 @@ window.LiveReloadOptions = {
     const replaceHead = isHTML && opts.headHtml;
     const doIndex = isHTML && opts.indexer && url.indexOf('.plain.html') < 0;
 
+    if (ctx.log.level === 'silly') {
+      ctx.log.trace(`[${id}] -----------------------------`);
+      ctx.log.trace(`[${id}] < http/${ret.httpVersion} ${ret.status} ${ret.statusText}`);
+      Object.entries(ret.headers.raw()).forEach(([name, value]) => {
+        ctx.log.trace(`[${id}] < ${name}: ${value}`);
+      });
+    }
+
     if (isHTML) {
-      let respBody;
-      let textBody;
-      if (contentType.startsWith('text/')) {
-        textBody = await ret.text();
-      } else {
-        respBody = await ret.buffer();
-      }
-      const lines = ['----------------------------->'];
+      let textBody = await ret.text();
       if (ctx.log.level === 'silly') {
-        lines.push(`${req.method} ${url}`);
-        Object.entries(headers).forEach(([name, value]) => {
-          lines.push(`${name}: ${value}`);
-        });
-        lines.push('');
-        lines.push('<-----------------------------');
-        lines.push('');
-        lines.push(`http/${ret.httpVersion} ${ret.status} ${ret.statusText}`);
-        ret.headers.forEach((name, value) => {
-          lines.push(`${name}: ${value}`);
-        });
-        lines.push('');
-        if (respBody) {
-          lines.push(`<binary ${respBody.length} bytes>`);
-        } else {
-          lines.push(textBody);
-        }
-        ctx.log.trace(lines.join('\n'));
+        ctx.log.trace(textBody);
+        ctx.log.trace(`[${id}] <<<--------------------------`);
       }
       if (replaceHead) {
         await opts.headHtml.setCookie(req.headers.cookie);
@@ -344,7 +331,7 @@ window.LiveReloadOptions = {
           url,
           opts.cacheDirectory,
           {
-            body: respBody || textBody,
+            body: textBody,
             headers: respHeaders,
             status: ret.status,
           },
@@ -353,12 +340,16 @@ window.LiveReloadOptions = {
       }
 
       res
-        .status(ret.status)
         .set(respHeaders)
-        .send(respBody || textBody);
+        .status(ret.status)
+        .send(textBody);
       return;
     }
 
+    if (ctx.log.level === 'silly') {
+      ctx.log.trace(`[${id}] <<<--------------------------`);
+    }
+
     if (opts.cacheDirectory) {
       const buffer = await ret.buffer();
       await utils.writeToCache(