From c8663f96364c56b723b7c61c79cb1e9d965bafee Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 26 Mar 2020 09:16:52 -0400 Subject: [PATCH 1/3] fix: add type checking in propagators --- .../opentelemetry-core/src/context/propagation/B3Propagator.ts | 3 ++- .../src/context/propagation/HttpTraceContext.ts | 3 ++- .../src/JaegerHttpTracePropagator.ts | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts b/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts index b3c971b87f..1aa3e08c7e 100644 --- a/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts +++ b/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts @@ -66,7 +66,6 @@ export class B3Propagator implements HttpTextPropagator { const traceIdHeader = getter(carrier, X_B3_TRACE_ID); const spanIdHeader = getter(carrier, X_B3_SPAN_ID); const sampledHeader = getter(carrier, X_B3_SAMPLED); - if (!traceIdHeader || !spanIdHeader) return context; const traceId = Array.isArray(traceIdHeader) ? traceIdHeader[0] : traceIdHeader; @@ -75,6 +74,8 @@ export class B3Propagator implements HttpTextPropagator { ? sampledHeader[0] : sampledHeader; + if (typeof traceId !== 'string' || typeof spanId !== 'string') return context; + if (isValidTraceId(traceId) && isValidSpanId(spanId)) { return setExtractedSpanContext(context, { traceId, diff --git a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts index d8b78d0fc2..6f278dda85 100644 --- a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts +++ b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts @@ -84,6 +84,7 @@ export class HttpTraceContext implements HttpTextPropagator { const traceParent = Array.isArray(traceParentHeader) ? traceParentHeader[0] : traceParentHeader; + if (typeof traceParent !== "string") return context; const spanContext = parseTraceParent(traceParent); if (!spanContext) return context; @@ -96,7 +97,7 @@ export class HttpTraceContext implements HttpTextPropagator { const state = Array.isArray(traceStateHeader) ? traceStateHeader.join(',') : traceStateHeader; - spanContext.traceState = new TraceState(state as string); + spanContext.traceState = new TraceState(typeof state === 'string' ? state : undefined); } return setExtractedSpanContext(context, spanContext); } diff --git a/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts b/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts index 13901fc8a1..a5ab840dc7 100644 --- a/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts +++ b/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts @@ -71,11 +71,12 @@ export class JaegerHttpTracePropagator implements HttpTextPropagator { extract(context: Context, carrier: unknown, getter: GetterFunction): Context { const uberTraceIdHeader = getter(carrier, this._jaegerTraceHeader); - if (!uberTraceIdHeader) return context; const uberTraceId = Array.isArray(uberTraceIdHeader) ? uberTraceIdHeader[0] : uberTraceIdHeader; + if (typeof uberTraceId !== 'string') return context; + const spanContext = deserializeSpanContext(uberTraceId); if (!spanContext) return context; From ffa02bbcd106aeaff773fd286081437508ef3d78 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 26 Mar 2020 09:37:21 -0400 Subject: [PATCH 2/3] chore: lint --- .../src/context/propagation/B3Propagator.ts | 3 ++- .../src/context/propagation/HttpTraceContext.ts | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts b/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts index 1aa3e08c7e..ce68edc3e2 100644 --- a/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts +++ b/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts @@ -74,7 +74,8 @@ export class B3Propagator implements HttpTextPropagator { ? sampledHeader[0] : sampledHeader; - if (typeof traceId !== 'string' || typeof spanId !== 'string') return context; + if (typeof traceId !== 'string' || typeof spanId !== 'string') + return context; if (isValidTraceId(traceId) && isValidSpanId(spanId)) { return setExtractedSpanContext(context, { diff --git a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts index 6f278dda85..4a5d4d698b 100644 --- a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts +++ b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts @@ -84,7 +84,7 @@ export class HttpTraceContext implements HttpTextPropagator { const traceParent = Array.isArray(traceParentHeader) ? traceParentHeader[0] : traceParentHeader; - if (typeof traceParent !== "string") return context; + if (typeof traceParent !== 'string') return context; const spanContext = parseTraceParent(traceParent); if (!spanContext) return context; @@ -97,7 +97,9 @@ export class HttpTraceContext implements HttpTextPropagator { const state = Array.isArray(traceStateHeader) ? traceStateHeader.join(',') : traceStateHeader; - spanContext.traceState = new TraceState(typeof state === 'string' ? state : undefined); + spanContext.traceState = new TraceState( + typeof state === 'string' ? state : undefined + ); } return setExtractedSpanContext(context, spanContext); } From c3edb73982c920d79a646f8d245a31793974bed6 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 26 Mar 2020 13:36:37 -0400 Subject: [PATCH 3/3] test: add test for bad getter --- .../test/context/B3Propagator.test.ts | 22 +++++++++++++++++++ .../test/context/HttpTraceContext.test.ts | 22 +++++++++++++++++++ .../test/JaegerHttpTracePropagator.test.ts | 22 +++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/packages/opentelemetry-core/test/context/B3Propagator.test.ts b/packages/opentelemetry-core/test/context/B3Propagator.test.ts index 5cc05951c4..1eb6759c16 100644 --- a/packages/opentelemetry-core/test/context/B3Propagator.test.ts +++ b/packages/opentelemetry-core/test/context/B3Propagator.test.ts @@ -267,5 +267,27 @@ describe('B3Propagator', () => { assert.deepStrictEqual(extractedSpanContext, undefined, testCase); }); }); + + it('should fail gracefully on bad responses from getter', () => { + const ctx1 = b3Propagator.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => 1 // not a number + ); + const ctx2 = b3Propagator.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => [] // empty array + ); + const ctx3 = b3Propagator.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => undefined // missing value + ); + + assert.ok(ctx1 === Context.ROOT_CONTEXT); + assert.ok(ctx2 === Context.ROOT_CONTEXT); + assert.ok(ctx3 === Context.ROOT_CONTEXT); + }); }); }); diff --git a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts index c9f524ec37..c82e50b636 100644 --- a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts +++ b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts @@ -212,5 +212,27 @@ describe('HttpTraceContext', () => { assert.deepStrictEqual(extractedSpanContext, undefined, testCase); }); }); + + it('should fail gracefully on bad responses from getter', () => { + const ctx1 = httpTraceContext.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => 1 // not a number + ); + const ctx2 = httpTraceContext.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => [] // empty array + ); + const ctx3 = httpTraceContext.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => undefined // missing value + ); + + assert.ok(ctx1 === Context.ROOT_CONTEXT); + assert.ok(ctx2 === Context.ROOT_CONTEXT); + assert.ok(ctx3 === Context.ROOT_CONTEXT); + }); }); }); diff --git a/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts b/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts index d2fa0b95e8..4013e4e8cd 100644 --- a/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts +++ b/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts @@ -185,4 +185,26 @@ describe('JaegerHttpTracePropagator', () => { ); }); }); + + it('should fail gracefully on bad responses from getter', () => { + const ctx1 = jaegerHttpTracePropagator.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => 1 // not a number + ); + const ctx2 = jaegerHttpTracePropagator.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => [] // empty array + ); + const ctx3 = jaegerHttpTracePropagator.extract( + Context.ROOT_CONTEXT, + carrier, + (c, k) => undefined // missing value + ); + + assert.ok(ctx1 === Context.ROOT_CONTEXT); + assert.ok(ctx2 === Context.ROOT_CONTEXT); + assert.ok(ctx3 === Context.ROOT_CONTEXT); + }); });