Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

baggage header decoding corrupts base64 format #4892

Closed
lukiano opened this issue Oct 26, 2022 · 1 comment · Fixed by #4898
Closed

baggage header decoding corrupts base64 format #4892

lukiano opened this issue Oct 26, 2022 · 1 comment · Fixed by #4898
Labels
Bug Something isn't working

Comments

@lukiano
Copy link

lukiano commented Oct 26, 2022

Describe the bug

W3CBaggagePropagator uses java.net.URLDecoder to decode baggage values which may be percent-encoded as per https://w3c.github.io/baggage/#definition
Looking at that specification, the + character is a valid character. However, URLDecoder transforms it to a space character. In particular, this change corrupts baggage values that are Base64 encoded.

Steps to reproduce

package org.example;

import java.util.Base64;
import java.util.HashMap;
import java.util.Map;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.baggage.Baggage;
import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.TextMapGetter;
import io.opentelemetry.sdk.OpenTelemetrySdk;

public class Main {
    public static void main(String[] args) {

        TextMapGetter<Map<String, String>> getter = new TextMapGetter<>() {
            @Override
            public Iterable<String> keys(Map<String, String> carrier) {
                return carrier.keySet();
            }

            @Override
            public String get(Map<String, String> carrier, String key) {
                return carrier.get(key);
            }
        };

        String encodedInBase64 = "MngJIFNQIAkhCSIJIwkkCSUJJgknCSgJKQkqCSsJLAktCS4JLwozeAkwCTEJMgkzCTQJNQk2CTcJOAk5CToJOwk8CT0JPgk/CjR4CUAJQQlCCUMJRAlFCUYJRwlICUkJSglLCUwJTQlOCU8KNXgJUAlRCVIJUwlUCVUJVglXCVgJWQlaCVsJXAldCV4JXwo2eAlgCWEJYgljCWQJZQlmCWcJaAlpCWoJawlsCW0JbglvCjd4CXAJcQlyCXMJdAl1CXYJdwl4CXkJegl7CXwJfQl+CQo4eAkJCQkJCQkJCQkJCQkJCQkKOXgJCQkJCQkJCQkJCQkJCQkJCkF4CU5CU1AJwqEJwqIJwqMJwqQJwqUJwqYJwqcJwqgJwqkJwqoJwqsJwqwJU0hZCcKuCcKvCkJ4CcKwCcKxCcKyCcKzCcK0CcK1CcK2CcK3CcK4CcK5CcK6CcK7CcK8CcK9CcK+CcK/CkN4CcOACcOBCcOCCcODCcOECcOFCcOGCcOHCcOICcOJCcOKCcOLCcOMCcONCcOOCcOPCkR4CcOQCcORCcOSCcOTCcOUCcOVCcOWCcOXCcOYCcOZCcOaCcObCcOcCcOdCcOeCcOfCkV4CcOgCcOhCcOiCcOjCcOkCcOlCcOmCcOnCcOoCcOpCcOqCcOrCcOsCcOtCcOuCcOvCkZ4CcOwCcOxCcOyCcOzCcO0CcO1CcO2CcO3CcO4CcO5CcO6CcO7CcO8CcO9CcO+CcO/Cgo=";

        Map<String, String> headers = new HashMap<>();
        headers.put("baggage", "foo="+encodedInBase64);

        OpenTelemetry openTelemetry = OpenTelemetrySdk.builder()
                .setPropagators(ContextPropagators.create(W3CBaggagePropagator.getInstance()))
                .build();

        Context context = Context.current();
        Context contextWithBaggage = openTelemetry.getPropagators().getTextMapPropagator().extract(context, headers, getter);
        Baggage baggage = Baggage.fromContext(contextWithBaggage);
        String fooValue = baggage.getEntryValue("foo");
        System.out.println(encodedInBase64);
        System.out.println(fooValue);
        Base64.getDecoder().decode(fooValue);
    }
}

What did you expect to see?

encodedInBase64: MngJIFNQIAkhCSIJIwkkCSUJJgknCSgJKQkqCSsJLAktCS4JLwozeAkwCTEJMgkzCTQJNQk2CTcJOAk5CToJOwk8CT0JPgk/CjR4CUAJQQlCCUMJRAlFCUYJRwlICUkJSglLCUwJTQlOCU8KNXgJUAlRCVIJUwlUCVUJVglXCVgJWQlaCVsJXAldCV4JXwo2eAlgCWEJYgljCWQJZQlmCWcJaAlpCWoJawlsCW0JbglvCjd4CXAJcQlyCXMJdAl1CXYJdwl4CXkJegl7CXwJfQl+CQo4eAkJCQkJCQkJCQkJCQkJCQkKOXgJCQkJCQkJCQkJCQkJCQkJCkF4CU5CU1AJwqEJwqIJwqMJwqQJwqUJwqYJwqcJwqgJwqkJwqoJwqsJwqwJU0hZCcKuCcKvCkJ4CcKwCcKxCcKyCcKzCcK0CcK1CcK2CcK3CcK4CcK5CcK6CcK7CcK8CcK9CcK+CcK/CkN4CcOACcOBCcOCCcODCcOECcOFCcOGCcOHCcOICcOJCcOKCcOLCcOMCcONCcOOCcOPCkR4CcOQCcORCcOSCcOTCcOUCcOVCcOWCcOXCcOYCcOZCcOaCcObCcOcCcOdCcOeCcOfCkV4CcOgCcOhCcOiCcOjCcOkCcOlCcOmCcOnCcOoCcOpCcOqCcOrCcOsCcOtCcOuCcOvCkZ4CcOwCcOxCcOyCcOzCcO0CcO1CcO2CcO3CcO4CcO5CcO6CcO7CcO8CcO9CcO+CcO/Cgo=
       fooValue: MngJIFNQIAkhCSIJIwkkCSUJJgknCSgJKQkqCSsJLAktCS4JLwozeAkwCTEJMgkzCTQJNQk2CTcJOAk5CToJOwk8CT0JPgk/CjR4CUAJQQlCCUMJRAlFCUYJRwlICUkJSglLCUwJTQlOCU8KNXgJUAlRCVIJUwlUCVUJVglXCVgJWQlaCVsJXAldCV4JXwo2eAlgCWEJYgljCWQJZQlmCWcJaAlpCWoJawlsCW0JbglvCjd4CXAJcQlyCXMJdAl1CXYJdwl4CXkJegl7CXwJfQl CQo4eAkJCQkJCQkJCQkJCQkJCQkKOXgJCQkJCQkJCQkJCQkJCQkJCkF4CU5CU1AJwqEJwqIJwqMJwqQJwqUJwqYJwqcJwqgJwqkJwqoJwqsJwqwJU0hZCcKuCcKvCkJ4CcKwCcKxCcKyCcKzCcK0CcK1CcK2CcK3CcK4CcK5CcK6CcK7CcK8CcK9CcK CcK/CkN4CcOACcOBCcOCCcODCcOECcOFCcOGCcOHCcOICcOJCcOKCcOLCcOMCcONCcOOCcOPCkR4CcOQCcORCcOSCcOTCcOUCcOVCcOWCcOXCcOYCcOZCcOaCcObCcOcCcOdCcOeCcOfCkV4CcOgCcOhCcOiCcOjCcOkCcOlCcOmCcOnCcOoCcOpCcOqCcOrCcOsCcOtCcOuCcOvCkZ4CcOwCcOxCcOyCcOzCcO0CcO1CcO2CcO3CcO4CcO5CcO6CcO7CcO8CcO9CcO CcO/Cgo=

encodedInBase64 and fooValue should be the same string. Furthermore, Base64.getDecoder().decode(fooValue) should not fail.

What did you see instead?
fooValue as a space where encodedInBase64 has as a + character.
Base64.getDecoder().decode(fooValue); throws a java.lang.IllegalArgumentException: Illegal base64 character 20

What version and what artifacts are you using?

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>io.opentelemetry</groupId>
                <artifactId>opentelemetry-bom</artifactId>
                <version>1.19.0</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
        </dependencies>
    </dependencyManagement>

    <dependencies>
        <dependency>
            <groupId>io.opentelemetry</groupId>
            <artifactId>opentelemetry-api</artifactId>
        </dependency>
        <dependency>
            <groupId>io.opentelemetry</groupId>
            <artifactId>opentelemetry-sdk</artifactId>
        </dependency>
    </dependencies>

Environment
Compiler: (e.g., "JDK 18.0.2")
OS: (e.g., "Mac OS Monterey")

@lukiano lukiano added the Bug Something isn't working label Oct 26, 2022
@lmonkiewicz
Copy link
Contributor

I'm looking into it, and it looks like a custom decoder has to be written, since the URLDecoder uses a different set of characters than the specification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants