dart-lang / shelf

Web server middleware for Dart
https://pub.dev/packages/shelf
BSD 3-Clause "New" or "Revised" License
893 stars 125 forks source link

Invalid argument (requestedUri): handlerPath "/" and url "43" must combine to equal requestedUri path "443".: Instance of '_SimpleUri' #414

Closed yixiaco closed 1 month ago

yixiaco commented 4 months ago

1.Specific operations: Set up windows proxy server:

image

2.Then create http server

import 'package:shelf/shelf.dart' as shelf;
import 'package:shelf/shelf_io.dart' as shelf_io;
import 'package:shelf_proxy/shelf_proxy.dart' as proxy;

void main() async {
  var handler = const shelf.Pipeline().addHandler(
        (request) async {
          if(request.requestedUri.host == '127.0.0.1' && request.requestedUri.port == 8080) {
            return shelf.Response.ok('Hello, World!\n');
          }
          return proxy.proxyHandler(request.requestedUri).call(request);
        },
      );

  // Start the server
  var server = await shelf_io.serve(handler, '127.0.0.1', 8080);

  print('Proxy server running at http://127.0.0.1:8080');

}

3.Use Chrome browser to visit github.com

flutter: Proxy server running at http://127.0.0.1:8080
ERROR - 2024-02-19 23:41:13.763125
Error parsing request.
Invalid argument (requestedUri): handlerPath "/" and url "43" must combine to equal requestedUri path "443".: Instance of '_SimpleUri'
dart:async  _startMicrotaskLoop

I analyzed the source code and found that it was caused by this:

image

https://github.com/dart-lang/shelf/assets/30982989/f8b74e79-5a62-44b1-91cc-347b20c1a8dd

insinfo commented 1 month ago

I'm also seeing this problem, but with webdev 3.4.0 when the internet goes down

kevmoo commented 1 month ago

@lrhn – Did you do _SimpleUri?

lrhn commented 1 month ago

I did, and this is working as intended.

The URI github.com:443 has scheme github.com and path 443 (. is a valid character valid in schemes). It has no authority part (host name, etc.) because an authority part must start with //.

The author probably intended to get what they'd get by writing Uri.parse('https://github.com:443/'), or maybe using Uri.https.

The request.dart code seems to assume that a URI path always starts with / (the .substring(1) intends to remove it). That's not necessarily true for a URI with no authority.

kevmoo commented 1 month ago

Thanks for clarifying, @lrhn !

lrhn commented 1 month ago

That does mean that the shelf request.dart file should probably be fixed. Line 352 can be changed to use, fx, .pathSegments.join('/') instead of .path.substring(1), like some other code in the file already does.

I'd also check if line 340 can be made to pass -1 as the first argument to substring.

That, or require paths to be absolute or there to be an authority component (explicit or implicit).

kevmoo commented 1 month ago

@lrhn – could you clarify a bit more here? I'm not sure I follow. Sounds like we should keep this open? Or file a new issue?

lrhn commented 1 month ago

The issue here may be using a weird looking URI by accident, but it is a syntactically valid URI. The code in this package assumes that a URI of that form cannot occur, which is why it does .path.substring(1); // Remove leading '/'. That assumption is wrong, and the example here shows such a URI reaching that code and removing a leading 4.

We should either fix the code to not make unwarranted assumptions, or document and enforce that not all URIs are supported valid arguments, so that a URI with a path without a leading / won't reach that code.

(Not sure it needs to be this issue, a more directly focused issue is probably easier to act on.)

Reprevise commented 3 weeks ago

Not sure what the ramifications of this issue are, like would end users who are requesting a normal endpoint see an issue? Or this this only happening for certain malformed URLs? For context, I'm running an API endpoint using shelf and I'm getting the exact same issue.

Guess it also doesn't help that _SimpleUri doesn't have a toString() implementation given the "Instance of '_SimpleUri'" string.

lrhn commented 3 weeks ago

_SimpleUri has a toString, but the error reporting code uses Error.safeToString on it to avoid risking more errors while reporting errors.

The issue should only happen for certain "specially-formed" URIs. They're not malformed from the URI syntax perspective, but are maybe not what the author intended. Can you say which URIs you get the issue for? (If you catch the error, you may be able to get to the Uri object inside and print its toString.)

Reprevise commented 3 weeks ago

Where should I catch the error?

lrhn commented 3 weeks ago

I'll admit I have no idea about the code of Shelf, just Uri, so I can't say where an appropriate place to catch errors would be (or if they can be caught at all).

kevmoo commented 3 weeks ago

Not sure if @natebosch or @devoncarew or @brianquinlan is interested in digging into the root cause here at all.