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

Server received pct-encoded password #1256

Open
idy opened this issue Jul 5, 2024 · 3 comments
Open

Server received pct-encoded password #1256

idy opened this issue Jul 5, 2024 · 3 comments
Labels
package:http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@idy
Copy link

idy commented Jul 5, 2024

When using the following code to send a request, the server receives the password not as ^pwd but as %5Epwd.

http.get(Uri.parse('https://user:%[email protected]'))
@idy idy added package:http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 5, 2024
@idy idy changed the title The server received pct-encoded password Server received pct-encoded password Jul 5, 2024
@lrhn
Copy link
Member

lrhn commented Jul 5, 2024

Does look like both dart:io and package:http sends the authentication as (code from dart:io)

String auth = base64Encode(utf8.encode(uri.userInfo));

and prefixed with "Basic ".

The problem is that uri.userInfo does not decode percent-escapes, so the code should be doing

String auth = base64Encode(utf8.encode(Uri.decodeComponent(uri.userInfo)));

Alterantively, the userInfo getter should decode for you. That's what most other getters on Uri do, extracting the meaning of the substring of the URI text, not its literal text.
(We should decide on one of those, doing both would be wrong too.)

EDIT: I figured out why we're doing what we're doing, so we're going to keep doing that. Do not expect a change to Uri.

I can't say what a browser would do, because my browser seems to ignore username/password in the request.
Probably for safety reasons.

@idy
Copy link
Author

idy commented Jul 5, 2024

As we discussed in lang/sdk#56114, I personally believe that Uri.parse should not store pct-encoded values in userInfo. Instead, it should encode these values when constructing the URL in toString().

@lrhn
Copy link
Member

lrhn commented Jul 16, 2024

Dart Uris store a valid (RFC-3986) URI internally, so toString is trivial, and then decodes when accessing individual parts if needed.
The most common use-case for URIs is to parse one, and then use it again directly. Having to do encoding in toString makes that more expensive.

Storing invalid content is possible. It may make it harder to determine where to cut, when extracting individual properties (but probably not, if we couldn't decide that at the start, it wouldn't have parsed at all.)

It may be confusing that the current implementation doesn't decode when accessing userInfo, but it's most likely deliberate, because userInfo is structured and can contain a single :.
If it also contains a %3A (aka encoded :), then decoding before returning the string would be misleading.
(Nobody does that, I know, but the design is intended to be defensive.)

That is:

void main() {
  var uri = Uri.parse("http://banana%3Acoop:[email protected]");
  print(uri);
  print(uri.userInfo);
  var [name, pwd] = uri.userInfo.split(':'); // Splits on the one `:`
  print(Uri.decodeComponent(name)); // Decodes into containing `:`
  print(Uri.decodeComponent(pwd));
  print(Uri.decodeComponent(uri.userInfo)); // Too late to see which `:`.
}

This code works and allows a : to be contained in the username (or password, which is possibly more relevant, those are more likely to contain non-alphanumeric characters.)

Because of that, it's not a good idea to decode userInfo on access, it can break code that properly escapes characters inside the userInfo, and uses the same characters as delimiters.
That means changing the behavior of Uri is unlikely to happen. (Unless we can do a complete rewrite and make it WhatWG URL compatible. I do want that, but it's a major change.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

2 participants