fix: add type checking in propagators (#904)

open-telemetry · Mar 27, 2020 · 47212de · 47212de
1 parent c15f360
commit 47212de
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 3 deletions.
diff --git 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,9 @@ 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
@@ -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,9 @@ 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-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
@@ -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/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;
 

diff --git 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);
+  });
 });