Skip to content

Commit

Permalink
fix: handle invalid chars in host header (#3923)
Browse files Browse the repository at this point in the history
  • Loading branch information
david-luna committed Mar 20, 2024
1 parent ca0e5b6 commit 0ff46e4
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
20 changes: 19 additions & 1 deletion CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,25 @@ Notes:
See the <<upgrade-to-v4>> guide.
==== Unreleased
[float]
===== Breaking changes
[float]
===== Features
[float]
===== Bug fixes
* Fix path resolution for requests that contain invalid characters in its
host header. ({pull}3923[#3923])
* Fix span names for `getMore` command of mongodb. ({pull}3919[#3919])
[float]
===== Chores
[[release-notes-4.5.0]]
==== 4.5.0 - 2024/03/13
Expand All @@ -46,7 +65,6 @@ See the <<upgrade-to-v4>> guide.
[float]
===== Bug fixes
* Fix span names for `getMore` command of mongodb. ({pull}3919[#3919])
* Fix instrumentation of mongodb to not break mongodb@6.4.0. Mongodb v6.4.0
included changes that resulted in the APM agent's instrumentation breaking it.
({pull}3897[#3897])
Expand Down
15 changes: 14 additions & 1 deletion lib/instrumentation/express-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,23 @@ function getPathFromRequest(req, useBase, usePathAsTransactionName) {
//
// Assuming 'http://' for the `base` URL is fine, because we don't use the
// protocol.
const base = 'http://' + (req.headers && req.headers.host);
let base;
try {
// Host header may contain invalid characters therefore the URL
// parsing will fail and break the app. This try block is to avoid it
// Ref: https://github.com/elastic/apm-agent-nodejs/issues/3874
const url = new url.URL('http://' + (req.headers && req.headers.host));
base = 'http://' + url.hostname;
} catch (err) {
base = 'http://undefined';
}

// We may receive invalid chars in the path also but the URL
// constructor escapes them without throwing.
const parsed = req.url.startsWith('/')
? new url.URL(base + req.url)
: new url.URL(req.url, base);

return parsed && parsed.pathname;
}
}
Expand Down
10 changes: 10 additions & 0 deletions test/instrumentation/express-utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ test('#getPathFromRequest', function (t) {
t.equals(path, '/foo/bar');
t.end();
});

t.test('should return path for an invalid host header', function (t) {
const req = createRequest(
'https://test.com/foo/bar?query=value#hash',
'invalid[hostname',
);
const path = getPathFromRequest(req, false, true);
t.equals(path, '/foo/bar');
t.end();
});
});

function createRequest(url, host = 'example.com') {
Expand Down

0 comments on commit 0ff46e4

Please sign in to comment.