From db413ee712d356321c0e3c9dddeb37badb2206b6 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 22 Oct 2024 21:52:02 +0200 Subject: [PATCH 1/8] fix(instrumentation-http): drop url.parse in favor of URL constructor --- .../src/http.ts | 16 +- .../src/utils.ts | 142 +++++++++++++++--- .../test/functionals/http-enable.test.ts | 39 ++++- .../test/functionals/https-enable.test.ts | 27 +++- 4 files changed, 196 insertions(+), 28 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 98a7ea2e04a..e6deed8cf8c 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -757,7 +757,10 @@ export class HttpInstrumentation extends InstrumentationBase original.apply(this, [optionsParsed, ...args]), + () => { + if (invalidUrl) { + // we know that the url is invalid, there's no point in injecting context as it will fail validation. + // Passing in what the user provided will give the user an error that matches what they'd see without + // the instrumentation. + return original.apply(this, [options, ...args]); + } else { + return original.apply(this, [optionsParsed, ...args]); + } + }, error => { if (error) { setSpanWithError(span, error, instrumentation._semconvStability); diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index a03600dfd42..9f8483193be 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -235,6 +235,64 @@ export const isCompressed = ( return !!encoding && encoding !== 'identity'; }; +/** + * Mimics Node.js conversion of URL strings to RequestOptions expected by + * `http.request` and `https.request` APIs. + * + * See https://github.com/nodejs/node/blob/2505e217bba05fc581b572c685c5cf280a16c5a3/lib/internal/url.js#L1415-L1437 + * + * @param stringUrl + * @throws TypeError if the URL is not valid. + */ +function stringUrlToHttpOptions( + stringUrl: string +): RequestOptions & { pathname: string } { + // This is heavily inspired by Node.js handling of the same situation, trying + // to follow it as closely as possible while keeping in mind that we only + // deal with string URLs, not URL objects. + const { + hostname, + pathname, + port, + username, + password, + search, + protocol, + hash, + href, + origin, + host, + } = new URL(stringUrl); + + const options: RequestOptions & { + pathname: string; + hash: string; + search: string; + href: string; + origin: string; + } = { + protocol: protocol, + hostname: + hostname && hostname[0] === '[' ? hostname.slice(1, -1) : hostname, + hash: hash, + search: search, + pathname: pathname, + path: `${pathname || ''}${search || ''}`, + href: href, + origin: origin, + host: host, + }; + if (port !== '') { + options.port = Number(port); + } + if (username || password) { + options.auth = `${decodeURIComponent(username)}:${decodeURIComponent( + password + )}`; + } + return options; +} + /** * Makes sure options is an url object * return an object with default value and parsed options @@ -248,14 +306,27 @@ export const getRequestInfo = ( origin: string; pathname: string; method: string; + invalidUrl: boolean; optionsParsed: RequestOptions; } => { - let pathname = '/'; - let origin = ''; + let pathname: string; + let origin: string; let optionsParsed: RequestOptions; + let invalidUrl = false; if (typeof options === 'string') { - optionsParsed = url.parse(options); - pathname = (optionsParsed as url.UrlWithStringQuery).pathname || '/'; + try { + const convertedOptions = stringUrlToHttpOptions(options); + optionsParsed = convertedOptions; + pathname = convertedOptions.pathname || '/'; + } catch (e) { + invalidUrl = true; + // for backward compatibility with how url.parse() behaved. + optionsParsed = { + path: options, + }; + pathname = optionsParsed.path || '/'; + } + origin = `${optionsParsed.protocol || 'http:'}//${optionsParsed.host}`; if (extraOptions !== undefined) { Object.assign(optionsParsed, extraOptions); @@ -285,16 +356,23 @@ export const getRequestInfo = ( { protocol: options.host ? 'http:' : undefined }, options ); - pathname = (options as url.URL).pathname; - if (!pathname && optionsParsed.path) { - pathname = url.parse(optionsParsed.path).pathname || '/'; - } + const hostname = optionsParsed.host || (optionsParsed.port != null ? `${optionsParsed.hostname}${optionsParsed.port}` : optionsParsed.hostname); origin = `${optionsParsed.protocol || 'http:'}//${hostname}`; + + pathname = (options as url.URL).pathname; + if (!pathname && optionsParsed.path) { + try { + const parsedUrl = new URL(optionsParsed.path, origin); + pathname = parsedUrl.pathname || '/'; + } catch (e) { + pathname = '/'; + } + } } // some packages return method in lowercase.. @@ -303,7 +381,7 @@ export const getRequestInfo = ( ? optionsParsed.method.toUpperCase() : 'GET'; - return { origin, pathname, method, optionsParsed }; + return { origin, pathname, method, optionsParsed, invalidUrl }; }; /** @@ -657,6 +735,23 @@ export function getRemoteClientAddress( return null; } +function tryGetUrlFromRequest( + component: 'http' | 'https', + request: IncomingMessage +): URL | undefined { + try { + // this is the way http.IncomingMessage.url docs recommend to handle this. + return new URL( + request.url ?? '/', + `${component}://${request.headers.host}` + ); + } catch (e) { + // something is wrong, use undefined + // TODO: log here? this should never happens AFAICT - if it ever does it may be good to have logs. + return undefined; + } +} + /** * Returns incoming request attributes scoped to the request data * @param {IncomingMessage} request the request object @@ -676,12 +771,12 @@ export const getIncomingRequestAttributes = ( const userAgent = headers['user-agent']; const ips = headers['x-forwarded-for']; const httpVersion = request.httpVersion; - const requestUrl = request.url ? url.parse(request.url) : null; - const host = requestUrl?.host || headers.host; - const hostname = - requestUrl?.hostname || - host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || - 'localhost'; + // TODO: is there a way host is not set? I don't think so + const host = headers.host; + let hostname = 'localhost'; + if (host != null && host !== '') { + hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1'); + } const method = request.method; const normalizedMethod = normalizeMethod(method); @@ -701,8 +796,10 @@ export const getIncomingRequestAttributes = ( [ATTR_USER_AGENT_ORIGINAL]: userAgent, }; - if (requestUrl?.pathname != null) { - newAttributes[ATTR_URL_PATH] = requestUrl.pathname; + const parsedUrl = tryGetUrlFromRequest(options.component, request); + + if (parsedUrl?.pathname != null) { + newAttributes[ATTR_URL_PATH] = parsedUrl.pathname; } if (remoteClientAddress != null) { @@ -719,11 +816,7 @@ export const getIncomingRequestAttributes = ( } const oldAttributes: Attributes = { - [SEMATTRS_HTTP_URL]: getAbsoluteUrl( - requestUrl, - headers, - `${options.component}:` - ), + [SEMATTRS_HTTP_URL]: parsedUrl?.toString(), [SEMATTRS_HTTP_HOST]: host, [SEMATTRS_NET_HOST_NAME]: hostname, [SEMATTRS_HTTP_METHOD]: method, @@ -738,8 +831,9 @@ export const getIncomingRequestAttributes = ( oldAttributes[SEMATTRS_HTTP_SERVER_NAME] = serverName; } - if (requestUrl) { - oldAttributes[SEMATTRS_HTTP_TARGET] = requestUrl.path || '/'; + if (parsedUrl?.pathname) { + oldAttributes[SEMATTRS_HTTP_TARGET] = + parsedUrl?.pathname + parsedUrl?.search || '/'; } if (userAgent !== undefined) { diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index f38d2018342..36e559017e2 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -349,6 +349,15 @@ describe('HttpInstrumentation', () => { assert.strictEqual(rpcData.route, undefined); rpcData.route = 'TheRoute'; } + if (request.url?.includes('/login')) { + assert.strictEqual( + request.headers.authorization, + 'Basic ' + Buffer.from('username:password').toString('base64') + ); + } + if (request.url?.includes('/withQuery')) { + assert.match(request.url, /withQuery\?foo=bar$/); + } response.end('Test Server Response'); }); @@ -596,7 +605,7 @@ describe('HttpInstrumentation', () => { assert.strictEqual(spans.length, 2); }); - for (const arg of ['string', {}, new Date()]) { + for (const arg of [{}, new Date()]) { it(`should be traceable and not throw exception in ${protocol} instrumentation when passing the following argument ${JSON.stringify( arg )}`, async () => { @@ -1004,6 +1013,34 @@ describe('HttpInstrumentation', () => { assert.deepStrictEqual(warnMessages, []); }); + + it('should not throw with cyrillic characters in the request path', async () => { + // see https://github.com/open-telemetry/opentelemetry-js/issues/5060 + await httpRequest.get(`${protocol}://${hostname}:${serverPort}/привет`); + }); + + it('should keep username and password in the request', async () => { + await httpRequest.get( + `${protocol}://username:password@${hostname}:${serverPort}/login` + ); + }); + + it('should keep query in the request', async () => { + await httpRequest.get( + `${protocol}://${hostname}:${serverPort}/withQuery?foo=bar` + ); + }); + + it('using an invalid url does throw from client but still creates a span', async () => { + try { + await httpRequest.get(`http://instrumentation.test:string-as-port/`); + } catch (e) { + assert.match(e.message, /Invalid URL/); + } + + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + }); }); describe('with semconv stability set to http', () => { diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts index 7c2e68bb0f7..a9f3b506a60 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts @@ -435,7 +435,7 @@ describe('HttpsInstrumentation', () => { assert.strictEqual(spans.length, 0); }); - for (const arg of ['string', {}, new Date()]) { + for (const arg of [{}, new Date()]) { it(`should be traceable and not throw exception in ${protocol} instrumentation when passing the following argument ${JSON.stringify( arg )}`, async () => { @@ -672,6 +672,31 @@ describe('HttpsInstrumentation', () => { }); req.end(); }); + + it('should keep username and password in the request', async () => { + await httpsRequest.get( + `${protocol}://username:password@${hostname}:${serverPort}/login` + ); + }); + + it('should keep query in the request', async () => { + await httpsRequest.get( + `${protocol}://${hostname}:${serverPort}/withQuery?foo=bar` + ); + }); + + it('using an invalid url does throw from client but still creates a span', async () => { + try { + await httpsRequest.get( + `${protocol}://instrumentation.test:string-as-port/` + ); + } catch (e) { + assert.match(e.message, /Invalid URL/); + } + + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + }); }); describe('partially disable instrumentation', () => { From 2df4ee1bdd2c4d156d86b6699ccd1d1f4aae9a88 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 25 Oct 2024 14:50:31 +0200 Subject: [PATCH 2/8] fixup! fix(instrumentation-http): drop url.parse in favor of URL constructor --- .../opentelemetry-instrumentation-http/src/utils.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 9f8483193be..758f609a367 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -771,12 +771,8 @@ export const getIncomingRequestAttributes = ( const userAgent = headers['user-agent']; const ips = headers['x-forwarded-for']; const httpVersion = request.httpVersion; - // TODO: is there a way host is not set? I don't think so const host = headers.host; - let hostname = 'localhost'; - if (host != null && host !== '') { - hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1'); - } + const hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || 'localhost'; const method = request.method; const normalizedMethod = normalizeMethod(method); From a88ef32058f3ad38652c26c43502400b143cffef Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 25 Oct 2024 15:56:42 +0200 Subject: [PATCH 3/8] fixup! fix(instrumentation-http): drop url.parse in favor of URL constructor --- .../src/utils.ts | 47 ++++++++++++++----- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 758f609a367..9501c5098b9 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -19,6 +19,7 @@ import { Span, context, SpanKind, + diag, } from '@opentelemetry/api'; import { ATTR_CLIENT_ADDRESS, @@ -79,6 +80,10 @@ import { } from './types'; import forwardedParse = require('forwarded-parse'); +const logger = diag.createComponentLogger({ + namespace: '@opentelemetry/instrumentation-http', +}); + /** * Get an absolute url */ @@ -735,20 +740,38 @@ export function getRemoteClientAddress( return null; } -function tryGetUrlFromRequest( +function getInfoFromIncomingMessage( component: 'http' | 'https', request: IncomingMessage -): URL | undefined { +): { pathname?: string; search?: string; toString: () => string } { try { - // this is the way http.IncomingMessage.url docs recommend to handle this. - return new URL( - request.url ?? '/', - `${component}://${request.headers.host}` - ); + if (request.headers.host) { + return new URL( + request.url ?? '/', + `${component}://${request.headers.host}` + ); + } else { + const unsafeParsedUrl = new URL( + request.url ?? '/', + // using localhost as a workaround to still use the URL constructor for parsing + `${component}://localhost` + ); + // since we use localhost as a workaround, ensure we hide the rest of the properties to avoid + // our workaround leaking though. + return { + pathname: unsafeParsedUrl.pathname, + search: unsafeParsedUrl.search, + toString: function () { + // we cannot use the result of unsafeParsedUrl.toString as it's potentially wrong. + return unsafeParsedUrl.pathname + unsafeParsedUrl.search; + }, + }; + } } catch (e) { - // something is wrong, use undefined - // TODO: log here? this should never happens AFAICT - if it ever does it may be good to have logs. - return undefined; + // something is wrong, use undefined - this *should* never happen, logging + // for troubleshooting in case it does happen. + logger.verbose('Unable to get URL from request', e); + return {}; } } @@ -792,7 +815,7 @@ export const getIncomingRequestAttributes = ( [ATTR_USER_AGENT_ORIGINAL]: userAgent, }; - const parsedUrl = tryGetUrlFromRequest(options.component, request); + const parsedUrl = getInfoFromIncomingMessage(options.component, request); if (parsedUrl?.pathname != null) { newAttributes[ATTR_URL_PATH] = parsedUrl.pathname; @@ -812,7 +835,7 @@ export const getIncomingRequestAttributes = ( } const oldAttributes: Attributes = { - [SEMATTRS_HTTP_URL]: parsedUrl?.toString(), + [SEMATTRS_HTTP_URL]: parsedUrl.toString(), [SEMATTRS_HTTP_HOST]: host, [SEMATTRS_NET_HOST_NAME]: hostname, [SEMATTRS_HTTP_METHOD]: method, From 64c6b12c57c7551ca7640de6869d3b0c6e98cb1e Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 25 Oct 2024 16:15:38 +0200 Subject: [PATCH 4/8] chore: add changelog entry --- experimental/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index aad560d8fca..53ba7ef7ead 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -11,6 +11,9 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) +* fix(instrumentation-http): drop url.parse in favor of URL constructor [#5091](https://github.com/open-telemetry/opentelemetry-js/pull/5091) @pichlermarc + * fixes a bug where using cyrillic characters in a client request string URL would throw + ### :books: (Refine Doc) ### :house: (Internal) From b0d1c8c0f7a21e23fc47ad817ac8068bd6513aa8 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 5 Nov 2024 13:58:25 +0100 Subject: [PATCH 5/8] chore: update changelog --- experimental/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 53ba7ef7ead..f553631bbf9 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -12,7 +12,7 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) * fix(instrumentation-http): drop url.parse in favor of URL constructor [#5091](https://github.com/open-telemetry/opentelemetry-js/pull/5091) @pichlermarc - * fixes a bug where using cyrillic characters in a client request string URL would throw + * fixes a bug where using cyrillic characters in a client request string URL would throw an exception, whereas an un-instrumented client would accept the same input without throwing an exception ### :books: (Refine Doc) From 9f9e97279ebc5dadcb352567b8cbcf724ec81e22 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 5 Nov 2024 14:17:05 +0100 Subject: [PATCH 6/8] refactor(instrumentation-http): pass instrumentation's logger to utils --- .../src/http.ts | 22 +++++++++------- .../src/utils.ts | 18 +++++++------ .../test/functionals/utils.test.ts | 25 +++++++++++++------ 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index e6deed8cf8c..6ae4de6ab19 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -632,15 +632,19 @@ export class HttpInstrumentation extends InstrumentationBase string } { try { if (request.headers.host) { @@ -788,7 +785,8 @@ export const getIncomingRequestAttributes = ( serverName?: string; hookAttributes?: Attributes; semconvStability: SemconvStability; - } + }, + logger: DiagLogger ): Attributes => { const headers = request.headers; const userAgent = headers['user-agent']; @@ -815,7 +813,11 @@ export const getIncomingRequestAttributes = ( [ATTR_USER_AGENT_ORIGINAL]: userAgent, }; - const parsedUrl = getInfoFromIncomingMessage(options.component, request); + const parsedUrl = getInfoFromIncomingMessage( + options.component, + request, + logger + ); if (parsedUrl?.pathname != null) { newAttributes[ATTR_URL_PATH] = parsedUrl.pathname; diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index 2d9911c7d37..2fe6187e4c4 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -19,6 +19,7 @@ import { SpanKind, context, Span, + diag, } from '@opentelemetry/api'; import { SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, @@ -432,10 +433,14 @@ describe('Utility', () => { 'user-agent': 'chrome', 'x-forwarded-for': ', , ', }; - const attributes = utils.getIncomingRequestAttributes(request, { - component: 'http', - semconvStability: SemconvStability.OLD, - }); + const attributes = utils.getIncomingRequestAttributes( + request, + { + component: 'http', + semconvStability: SemconvStability.OLD, + }, + diag + ); assert.strictEqual(attributes[SEMATTRS_HTTP_ROUTE], undefined); }); @@ -448,10 +453,14 @@ describe('Utility', () => { request.headers = { 'user-agent': 'chrome', }; - const attributes = utils.getIncomingRequestAttributes(request, { - component: 'http', - semconvStability: SemconvStability.OLD, - }); + const attributes = utils.getIncomingRequestAttributes( + request, + { + component: 'http', + semconvStability: SemconvStability.OLD, + }, + diag + ); assert.strictEqual(attributes[SEMATTRS_HTTP_TARGET], '/user/?q=val'); }); }); From 4c5825ca6d2a9da90a9defcb38e2e3117280524e Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 6 Nov 2024 14:35:49 +0100 Subject: [PATCH 7/8] fix(instrumentation-http)!: verbose log if URL passed to http request cannot be parsed --- .../packages/opentelemetry-instrumentation-http/src/http.ts | 1 + .../opentelemetry-instrumentation-http/src/utils.ts | 6 ++++++ .../test/functionals/utils.test.ts | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 6ae4de6ab19..fb852c329ea 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -762,6 +762,7 @@ export class HttpInstrumentation extends InstrumentationBase { urlParsedWithUndefinedHostAndNullPort, whatWgUrl, ]) { - const result = utils.getRequestInfo(param); + const result = utils.getRequestInfo(diag, param); assert.strictEqual(result.optionsParsed.hostname, 'google.fr'); assert.strictEqual(result.optionsParsed.protocol, 'http:'); assert.strictEqual(result.optionsParsed.path, '/aPath?qu=ry'); From 768fdff906e9ff0f3ee7098a119541598343dc45 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 6 Nov 2024 14:41:49 +0100 Subject: [PATCH 8/8] chore: update changelog to reflect breaking change in getRequestInfo() signature --- experimental/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 82675e119a0..795ee0f3f5e 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -7,6 +7,9 @@ All notable changes to experimental packages in this project will be documented ### :boom: Breaking Change +* fix(instrumentation-http): drop url.parse in favor of URL constructor [#5091](https://github.com/open-telemetry/opentelemetry-js/pull/5091) @pichlermarc + * (user-facing): signature of `getRequestInfo()` now requires a `DiagLogger` to be passed at the first position + ### :rocket: (Enhancement) ### :bug: (Bug Fix)