Skip to content

Commit f3b0bd6

Browse files
authored
Handle cases where colon is in the first path segment (#130)
`Request.url.path` is supposed to be relative to `Request.handlerPath`, this invariant is enforced by throwing `ArgumentError` in the constructor. However, if an incoming request has colon in the first path segment the `shelf_io` adapter will create a request with `/` handler path creating a `Request.url.path` that is relative to `/` and which has colon in the first segment. This causes unnecessary encoding of the colon which breaks the invariant check. In hindsight it's probably not ideal to use `Uri` for holding a relative path as this will always mangle the colon. But fixing this is very breaking.
1 parent fdce08a commit f3b0bd6

File tree

3 files changed

+48
-3
lines changed

3 files changed

+48
-3
lines changed

lib/src/request.dart

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:convert';
66

7+
import 'package:collection/collection.dart';
78
import 'package:http_parser/http_parser.dart';
89
import 'package:stream_channel/stream_channel.dart';
910

@@ -178,11 +179,29 @@ class Request extends Message {
178179
requestedUri, 'requestedUri', 'may not have a fragment.');
179180
}
180181

181-
if (this.handlerPath + this.url.path != this.requestedUri.path) {
182+
// Notice that because relative paths must encode colon (':') as %3A we
183+
// cannot actually combine this.handlerPath and this.url.path, but we can
184+
// compare the pathSegments. In practice exposing this.url.path as a Uri
185+
// and not a String is probably the underlying flaw here.
186+
final handlerPathSegments = Uri(path: this.handlerPath).pathSegments;
187+
// If there is a last pathSegment and it is empty we remove it because it
188+
// is only present to retain a trailing slash.
189+
Iterable<String> pathSegments = handlerPathSegments;
190+
if (handlerPathSegments.isNotEmpty && handlerPathSegments.last.isEmpty) {
191+
pathSegments = handlerPathSegments.getRange(
192+
0,
193+
handlerPathSegments.length - 1,
194+
);
195+
}
196+
pathSegments = pathSegments.followedBy(this.url.pathSegments);
197+
if (!IterableEquality().equals(
198+
pathSegments,
199+
this.requestedUri.pathSegments,
200+
)) {
182201
throw ArgumentError.value(
183202
requestedUri,
184203
'requestedUri',
185-
'handlerPath "$handlerPath" and url "$url" must '
204+
'handlerPath "${this.handlerPath}" and url "${this.url}" must '
186205
'combine to equal requestedUri path "${requestedUri.path}".');
187206
}
188207
}

test/request_test.dart

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,22 @@ void main() {
3333
expect(request.url, equals(Uri.parse('foo/bar?q=1')));
3434
});
3535

36+
test('may contain colon', () {
37+
var request = Request('GET', Uri.parse('http://localhost/foo/bar:42'));
38+
expect(request.url, equals(Uri.parse('foo/bar:42')));
39+
});
40+
41+
test('may contain colon in first segment', () {
42+
var request = Request('GET', Uri.parse('http://localhost/foo:bar/42'));
43+
expect(request.url, equals(Uri.parse('foo%3Abar/42')));
44+
});
45+
46+
test('may contain slash', () {
47+
var request =
48+
Request('GET', Uri.parse('http://localhost/foo/bar%2f42'));
49+
expect(request.url, equals(Uri.parse('foo/bar%2f42')));
50+
});
51+
3652
test('is inferred from handlerPath if possible', () {
3753
var request = Request('GET', Uri.parse('http://localhost/foo/bar?q=1'),
3854
handlerPath: '/foo/');

test/shelf_io_test.dart

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ void main() {
103103
expect(response.body, 'Hello from /foo/bar');
104104
});
105105

106+
test('Request can handle colon in first path segment', () async {
107+
await _scheduleServer(syncHandler);
108+
109+
var response = await _get(path: 'user:42');
110+
expect(response.statusCode, HttpStatus.ok);
111+
expect(response.body, 'Hello from /user:42');
112+
});
113+
106114
test('chunked requests are un-chunked', () async {
107115
await _scheduleServer(expectAsync1((request) {
108116
expect(request.contentLength, isNull);
@@ -563,11 +571,13 @@ Future _scheduleServer(Handler handler,
563571

564572
Future<http.Response> _get({
565573
Map<String, /* String | List<String> */ Object> headers,
574+
String path = '',
566575
}) async {
567576
// TODO: use http.Client once it supports sending and receiving multiple headers.
568577
final client = HttpClient();
569578
try {
570-
final rq = await client.getUrl(Uri.parse('http://localhost:$_serverPort/'));
579+
final rq =
580+
await client.getUrl(Uri.parse('http://localhost:$_serverPort/$path'));
571581
headers?.forEach((key, value) {
572582
rq.headers.add(key, value);
573583
});

0 commit comments

Comments
 (0)