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

Uri does not encode (or double-encodes) non-URL safe base64 path segments. #57015

Open
diegotori opened this issue Nov 1, 2024 · 4 comments
Open
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-documentation A request to add or improve documentation

Comments

@diegotori
Copy link

diegotori commented Nov 1, 2024

When attempting to create a Uri with a base64 encoded path segment value that is NOT URL safe (i.e containing illegal characters), it does not encode it (or double-encodes it when encoding it prior to creating the instance), when either creating a URI from scratch, or when replacing an existing one.

In other words, when encoding the base64 value using Uri.encodeComponent and creating the Uri using Uri.parse, it properly converts the unsafe == into properly encoded %3D%3D characters.

However, when creating the Uri either from its constructor, or when calling replace on an existing one, when placing an unencoded base64 value as a path segment, it returns == when calling toString on the resulting instance. Furthermore, when encoding the value using Uri.encodeComponent and placing it as a path segment, it double-encodes the already encoded %3D%3D value to %253D%253D instead.

Here is code that highlights this issue:

void main() {
  final baseUri = Uri(
    scheme: "https",
    host: "www.something.com",
    pathSegments: ["share"],
  );

  // Given an unsafe base64 URL value
  const unsafeBase64UrlValue = "c29tZSB2YWx1ZQ==";
  
  // When encoding it for use in a URL.
  final encoded = Uri.encodeComponent(unsafeBase64UrlValue);

  // Properly retains the parsed URL's encoded path segments.
  final properlyEncoded =
      Uri.parse("https://www.something.com/share/$encoded").replace(
    queryParameters: {
      "foo": "bar",
    },
  );
  print("Properly encoded base64 path URI: ${properlyEncoded.toString()}");

  // Does not encode the unsafe base64 path segment
  final unencodedBase64PathUri = baseUri.replace(
    pathSegments: [
      "share",
      unsafeBase64UrlValue,
    ],
    queryParameters: {
      "foo": "bar",
    },
  );
  print("Unencoded base64 path URI: ${unencodedBase64PathUri.toString()}");

  // Double encodes the encoded base64 path segment
  final doubleEncodedBase64PathUri = baseUri.replace(
    pathSegments: [
      "share",
      encoded,
    ],
    queryParameters: {
      "foo": "bar",
    },
  );
  print(
      "Double encoded base64 path URI: ${doubleEncodedBase64PathUri.toString()}");
}

Also available as a DartPad.

Currently running the following Dart version on macOS 14.5:

Dart SDK version: 3.5.3 (stable) (Wed Sep 11 16:22:47 2024 +0000) on "macos_arm64"
@dart-github-bot
Copy link
Collaborator

Summary: The Uri class does not properly encode or double-encodes non-URL safe base64 path segments when creating a new Uri instance or replacing an existing one. This results in incorrect URIs with unencoded or double-encoded path segments.

@dart-github-bot dart-github-bot added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Nov 1, 2024
@lrhn
Copy link
Member

lrhn commented Nov 1, 2024

The = character is a valid pchar, it's a sub-delimiter, so it doesn't need to be escaped.
That's why the replace function doesn't escape it.

The pathSegments argument is assumed to be unescaped source path segments (not pre-escaped URI path segments), so the % is (double) escaped.

The Uri.encodeComponent is used for query elements, like the name and value of ?name=value, so it escapes = characters, It doesn't know that the text will be used as a path segment where = would be OK.

So, all in all, working as intended.

@lrhn lrhn added closed-as-intended Closed as the reported issue is expected behavior library-core and removed triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 1, 2024
@lrhn lrhn closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2024
@diegotori
Copy link
Author

@lrhn I think it might make sense to point out this caveat in the existing documentation. Otherwise, other devs might get tripped up going forward.

@lrhn lrhn reopened this Nov 1, 2024
@lrhn lrhn added type-documentation A request to add or improve documentation and removed closed-as-intended Closed as the reported issue is expected behavior type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Nov 1, 2024
@lrhn
Copy link
Member

lrhn commented Nov 1, 2024

True, let's make it a documentation issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-documentation A request to add or improve documentation
Projects
None yet
Development

No branches or pull requests

3 participants