Return the correct HTTP status code for badly formatted requests. (da…

…rt-lang#112)

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.7.5
+
+* Return the correct HTTP status code for badly formatted requests.
+
 ## 0.7.4+1
 
 * Allow `stream_channel` version 2.x

lib/shelf_io.dart

@@ -73,9 +73,23 @@ Future handleRequest(HttpRequest request, Handler handler) async {
   Request shelfRequest;
   try {
     shelfRequest = _fromHttpRequest(request);
+  } on ArgumentError catch (error, stackTrace) {
+    if (error.name == 'method' || error.name == 'requestedUri') {
+      // TODO: use a reduced log level when using package:logging
+      _logTopLevelError('Error parsing request.\n$error', stackTrace);
+      final response = Response(400,
+          body: 'Bad Request',
+          headers: {HttpHeaders.contentTypeHeader: 'text/plain'});
+      await _writeResponse(response, request.response);
+    } else {
+      _logTopLevelError('Error parsing request.\n$error', stackTrace);
+      final response = Response.internalServerError();
+      await _writeResponse(response, request.response);
+    }
+    return;
   } catch (error, stackTrace) {
-    var response =
-        _logTopLevelError('Error parsing request.\n$error', stackTrace);
+    _logTopLevelError('Error parsing request.\n$error', stackTrace);
+    final response = Response.internalServerError();
     await _writeResponse(response, request.response);
     return;
   }
@@ -203,10 +217,11 @@ Response _logError(Request request, String message, [StackTrace stackTrace]) {
   buffer.writeln();
   buffer.write(message);
 
-  return _logTopLevelError(buffer.toString(), stackTrace);
+  _logTopLevelError(buffer.toString(), stackTrace);
+  return Response.internalServerError();
 }
 
-Response _logTopLevelError(String message, [StackTrace stackTrace]) {
+void _logTopLevelError(String message, [StackTrace stackTrace]) {
   var chain = Chain.current();
   if (stackTrace != null) {
     chain = Chain.forTrace(stackTrace);
@@ -218,5 +233,4 @@ Response _logTopLevelError(String message, [StackTrace stackTrace]) {
   stderr.writeln('ERROR - ${DateTime.now()}');
   stderr.writeln(message);
   stderr.writeln(chain);
-  return Response.internalServerError();
 }

lib/src/request.dart

@@ -167,20 +167,24 @@ class Request extends Message {
         this.handlerPath = _computeHandlerPath(requestedUri, handlerPath, url),
         this._onHijack = onHijack,
         super(body, encoding: encoding, headers: headers, context: context) {
-    if (method.isEmpty) throw ArgumentError('method cannot be empty.');
+    if (method.isEmpty)
+      throw ArgumentError.value(method, 'method', 'cannot be empty.');
 
     if (!requestedUri.isAbsolute) {
-      throw ArgumentError(
-          'requestedUri "$requestedUri" must be an absolute URL.');
+      throw ArgumentError.value(
+          requestedUri, 'requestedUri', 'must be an absolute URL.');
     }
 
     if (requestedUri.fragment.isNotEmpty) {
-      throw ArgumentError(
-          'requestedUri "$requestedUri" may not have a fragment.');
+      throw ArgumentError.value(
+          requestedUri, 'requestedUri', 'may not have a fragment.');
     }
 
     if (this.handlerPath + this.url.path != this.requestedUri.path) {
-      throw ArgumentError('handlerPath "$handlerPath" and url "$url" must '
+      throw ArgumentError.value(
+          requestedUri,
+          'requestedUri',
+          'handlerPath "$handlerPath" and url "$url" must '
           'combine to equal requestedUri path "${requestedUri.path}".');
     }
   }

pubspec.yaml

@@ -1,5 +1,5 @@
 name: shelf
-version: 0.7.4+1
+version: 0.7.5-dev
 description: >-
   A model for web server middleware that encourages composition and easy reuse
 author: Dart Team <[email protected]>

test/shelf_io_test.dart

@@ -267,7 +267,7 @@ void main() {
     expect(response.body, 'Hello from /');
   });
 
-  test('a bad HTTP request results in a 500 response', () async {
+  test('a bad HTTP host request results in a 500 response', () async {
     await _scheduleServer(syncHandler);
 
     var socket = await Socket.connect('localhost', _serverPort);
@@ -284,6 +284,21 @@ void main() {
         await utf8.decodeStream(socket), contains('500 Internal Server Error'));
   });
 
+  test('a bad HTTP URL request results in a 400 response', () async {
+    await _scheduleServer(syncHandler);
+    final socket = await Socket.connect('localhost', _serverPort);
+
+    try {
+      socket.write('GET /#/ HTTP/1.1\r\n');
+      socket.write('Host: localhost\r\n');
+      socket.write('\r\n');
+    } finally {
+      await socket.close();
+    }
+
+    expect(await utf8.decodeStream(socket), contains('400 Bad Request'));
+  });
+
   group('date header', () {
     test('is sent by default', () async {
       await _scheduleServer(syncHandler);