From 2df03e05305a8c4e3d537bbb6f637a4e4387b9b5 Mon Sep 17 00:00:00 2001 From: Tobias Bocanegra Date: Fri, 17 Nov 2023 17:44:37 +0100 Subject: [PATCH] fix: don't respond when headers already sent fixes #2276 --- src/server/HelixServer.js | 20 +++++++++++-- src/server/RequestContext.js | 12 ++++++++ src/server/utils.js | 55 +++++++++++++++--------------------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/server/HelixServer.js b/src/server/HelixServer.js index aab3b223c..7df9d3002 100644 --- 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 index 87c0edb02..b17e545ae 100644 --- 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 index eb6236656..e5ff00a50 100644 --- 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,7 +264,7 @@ 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()); @@ -271,11 +272,12 @@ window.LiveReloadOptions = { 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(``); - } 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(