Protect from exceptions in decodeURIComponent; do not url-encode span context

src/propagators/text_map_codec.js

@@ -40,22 +40,32 @@ export default class TextMapCodec {
         this._metrics = options.metrics || new Metrics(new NoopMetricFactory());
     }
 
-    _encodedValue(value: string): string {
+    _encodeValue(value: string): string {
         if (this._urlEncoding) {
             return encodeURIComponent(value);
         }
 
         return value;
     }
 
-    _decodedValue(value: string): string {
-        if (this._urlEncoding) {
-            return decodeURIComponent(value);
+    _decodeValue(value: string): string {
+        // only use url-decoding if there are meta-characters '%'
+        if (this._urlEncoding && value.indexOf('%') > -1) {
+            return this._decodeURIValue(value);
         }
 
         return value;
     }
 
+    _decodeURIValue(value: string): string {
+        // unfortunately, decodeURIComponent() can throw 'URIError: URI malformed' on bad strings
+        try {
+            return decodeURIComponent(value);
+        } catch (e) {
+            return value;
+        }
+    }
+
     extract(carrier: any): ?SpanContext {
         let spanContext = new SpanContext();
         let baggage = {};
@@ -65,19 +75,19 @@ export default class TextMapCodec {
             if (carrier.hasOwnProperty(key)) {
                 let lowerKey = key.toLowerCase();
                 if (lowerKey === this._contextKey) {
-                    let decodedContext = SpanContext.fromString(this._decodedValue(carrier[key]));
+                    let decodedContext = SpanContext.fromString(this._decodeValue(carrier[key]));
                     if (decodedContext === null) {
                         this._metrics.decodingErrors.increment(1);
                     } else {
                         spanContext = decodedContext;
                     }
                 } else if (lowerKey === constants.JAEGER_DEBUG_HEADER) {
-                    debugId = this._decodedValue(carrier[key]);
+                    debugId = this._decodeValue(carrier[key]);
                 } else if (lowerKey === constants.JAEGER_BAGGAGE_HEADER) {
-                    this._parseCommaSeparatedBaggage(baggage, this._decodedValue(carrier[key]));
+                    this._parseCommaSeparatedBaggage(baggage, this._decodeValue(carrier[key]));
                 } else if (Utils.startsWith(lowerKey, this._baggagePrefix)) {
                     let keyWithoutPrefix = key.substring(this._baggagePrefix.length);
-                    baggage[keyWithoutPrefix] = this._decodedValue(carrier[key]);
+                    baggage[keyWithoutPrefix] = this._decodeValue(carrier[key]);
                 }
             }
         }
@@ -89,12 +99,12 @@ export default class TextMapCodec {
 
     inject(spanContext: SpanContext, carrier: any): void {
         let stringSpanContext = spanContext.toString();
-        carrier[this._contextKey] = this._encodedValue(stringSpanContext);
+        carrier[this._contextKey] = stringSpanContext; // no need to encode this
 
         let baggage = spanContext.baggage;
         for (let key in baggage) {
             if (baggage.hasOwnProperty(key)) {
-                let value = this._encodedValue(spanContext.baggage[key]);
+                let value = this._encodeValue(spanContext.baggage[key]);
                 carrier[`${this._baggagePrefix}${key}`] = value;
             }
         }

test/propagators.js

@@ -0,0 +1,62 @@
+// Copyright (c) 2016 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+import {assert} from 'chai';
+import TextMapCodec from '../src/propagators/text_map_codec';
+import SpanContext from '../src/span_context';
+
+describe ('TextMapCodec', () => {
+    it('should not URL-decode value that has no % meta-characters', () => {
+        let codec = new TextMapCodec({ urlEncoding: true });
+        codec._decodeURIValue = (value: string) => {
+            throw new URIError('fake error');
+        };
+        assert.strictEqual(codec._decodeValue('abc'), 'abc');
+    });
+
+    it('should not throw exception on bad URL-encoded values', () => {
+        let codec = new TextMapCodec({ urlEncoding: true });
+        // this string throws exception when passed to decodeURIComponent
+        assert.strictEqual(codec._decodeValue('%EA'), '%EA');
+    });
+
+    it('should not URL-encode span context', () => {
+        let codec = new TextMapCodec({ urlEncoding: true, contextKey: 'trace-context' });
+        let ctx = SpanContext.fromString('1:1:1:1');
+        let out = {};
+        codec.inject(ctx, out);
+        assert.strictEqual(out['trace-context'], '1:1:1:1');
+    });
+
+    it('should decode baggage', () => {
+        let codec = new TextMapCodec({ 
+            urlEncoding: true, 
+            contextKey: 'trace-context',
+            baggagePrefix: 'baggage-'
+        });
+        let carrier = {
+            'trace-context': '1:1:1:1',
+            'baggage-some-key': 'some-value',
+            'garbage-in': 'garbage-out'
+        };
+        let ctx = codec.extract(carrier);
+        assert.deepEqual(ctx.baggage, { 'some-key': 'some-value' });
+    });
+});