fix(dart_frog): add default handler for disallowed http methods (#1) (#1727)

Author: felangel

packages/dart_frog/lib/src/request.dart

@@ -1,5 +1,22 @@
 part of '_internal.dart';
 
+/// {@template unsupported_http_method_exception}
+/// Exception thrown when an unsupported HTTP method is used.
+/// {@endtemplate}
+class UnsupportedHttpMethodException implements Exception {
+  /// {@macro unsupported_http_method_exception}
+  const UnsupportedHttpMethodException(this.method);
+
+  /// The unsupported http method.
+  final String method;
+
+  @override
+  String toString() => '''
+Unsupported HTTP method: $method. 
+The following methods are supported:
+${HttpMethod.values.map((m) => m.value.toUpperCase()).join(', ')}.''';
+}
+
 /// {@template request}
 /// An HTTP request.
 /// {@endtemplate}
@@ -112,7 +129,10 @@ class Request {
 
   /// The [HttpMethod] associated with the request.
   HttpMethod get method {
-    return HttpMethod.values.firstWhere((m) => m.value == _request.method);
+    return HttpMethod.values.firstWhere(
+      (m) => m.value == _request.method,
+      orElse: () => throw UnsupportedHttpMethodException(_request.method),
+    );
   }
 
   /// Returns a [Stream] representing the body.

packages/dart_frog/lib/src/router.dart

@@ -36,8 +36,14 @@ class Router {
   ///
   /// The [notFoundHandler] will be invoked for requests where no matching route
   /// was found. By default, a simple 404 response will be used.
-  Router({Handler notFoundHandler = _defaultNotFound})
-      : _notFoundHandler = notFoundHandler;
+  ///
+  /// The [methodNotAllowedHandler] will be invoked for requests where the HTTP
+  /// method is not allowed. By default, a simple 405 response will be used.
+  Router({
+    Handler notFoundHandler = _defaultNotFound,
+    Handler methodNotAllowedHandler = _defaultMethodNotAllowed,
+  })  : _notFoundHandler = notFoundHandler,
+        _methodNotAllowedHandler = methodNotAllowedHandler;
 
   /// Name of the parameter used for matching
   /// the rest of the path in a mounted route.
@@ -48,6 +54,7 @@ class Router {
 
   final List<RouterEntry> _routes = [];
   final Handler _notFoundHandler;
+  final Handler _methodNotAllowedHandler;
 
   /// Add [handler] for [verb] requests to [route].
   ///
@@ -167,8 +174,14 @@ class Router {
   /// This method allows a Router instance to be a [Handler].
   Future<Response> call(RequestContext context) async {
     for (final route in _routes) {
-      if (route.verb != context.request.method.value.toUpperCase() &&
-          route.verb != 'ALL') {
+      final HttpMethod method;
+      try {
+        method = context.request.method;
+      } on UnsupportedHttpMethodException {
+        return _methodNotAllowedHandler(context);
+      }
+
+      if (route.verb != method.value.toUpperCase() && route.verb != 'ALL') {
         continue;
       }
       final params = route.match('/${context.request._request.url.path}');
@@ -211,11 +224,22 @@ class Router {
 
   static Response _defaultNotFound(RequestContext context) => routeNotFound;
 
+  static Response _defaultMethodNotAllowed(RequestContext context) {
+    return methodNotAllowed;
+  }
+
   /// Sentinel [Response] object indicating that no matching route was found.
   ///
   /// This is the default response value from a [Router] created without a
   /// `notFoundHandler`, when no routes matches the incoming request.
   static final Response routeNotFound = _RouteNotFoundResponse();
+
+  /// Sentinel [Response] object indicating that the http method
+  /// was not allowed for the requested route.
+  ///
+  /// This is the default response value from a [Router] created without a
+  /// `methodNotAllowedHandler`, when an unsupported http method is requested.
+  static final Response methodNotAllowed = _MethodNotAllowedResponse();
 }
 
 /// Extends [Response] to allow it to be used multiple times in the
@@ -241,6 +265,29 @@ class _RouteNotFoundResponse extends Response {
   }
 }
 
+/// Extends [Response] to allow it to be used multiple times in the
+/// actual content being served.
+class _MethodNotAllowedResponse extends Response {
+  _MethodNotAllowedResponse()
+      : super(statusCode: HttpStatus.methodNotAllowed, body: _message);
+  static const _message = 'Method not allowed';
+  static final _messageBytes = utf8.encode(_message);
+
+  @override
+  shelf.Response get _response => super._response.change(body: _messageBytes);
+
+  @override
+  Stream<List<int>> bytes() => Stream<List<int>>.value(_messageBytes);
+
+  @override
+  Future<String> body() async => _message;
+
+  @override
+  Response copyWith({Map<String, Object?>? headers, dynamic body}) {
+    return super.copyWith(headers: headers, body: body ?? _message);
+  }
+}
+
 /// Check if the [regexp] is non-capturing.
 bool _isNoCapture(String regexp) {
   // Construct a new regular expression matching anything containing regexp,

packages/dart_frog/test/src/request_test.dart

@@ -3,6 +3,7 @@ import 'dart:convert';
 import 'dart:io';
 
 import 'package:dart_frog/dart_frog.dart';
+import 'package:dart_frog/src/_internal.dart';
 import 'package:dart_frog/src/body_parsers/body_parsers.dart';
 import 'package:test/test.dart';
 
@@ -43,6 +44,20 @@ void main() {
       expect(request.bytes(), emits(utf8.encode(body)));
     });
 
+    test('throw exception when method is unsupported', () {
+      final request = Request('FOO', localhost);
+      expect(
+        () => request.method,
+        throwsA(
+          isA<UnsupportedHttpMethodException>()
+              .having((e) => e.toString(), 'toString', '''
+Unsupported HTTP method: FOO. 
+The following methods are supported:
+${HttpMethod.values.map((m) => m.value.toUpperCase()).join(', ')}.'''),
+        ),
+      );
+    });
+
     test('throws exception when unable to read body', () async {
       final exception = Exception('oops');
       final body = Stream<Object>.error(exception);

packages/dart_frog/test/src/router_test.dart

@@ -91,6 +91,13 @@ void main() {
     expect(response.statusCode, equals(HttpStatus.notFound));
     expect(response.body, equals('Route not found'));
 
+    final streamedResponse = await http.Client().send(
+      http.Request('FOO', Uri.parse('${server.url}/hello/world')),
+    );
+    response = await http.Response.fromStream(streamedResponse);
+    expect(response.statusCode, equals(HttpStatus.methodNotAllowed));
+    expect(response.body, equals('Method not allowed'));
+
     response = await http.get(Uri.parse('${server.url}/hello/world'));
     expect(response.statusCode, equals(HttpStatus.ok));
     expect(response.body, equals('hello'));
@@ -445,6 +452,26 @@ void main() {
     expect(response.body(), completion(equals('Route not found')));
   });
 
+  test('can call Router.methodNotAllowed.body() multiple times', () async {
+    final b1 = await Router.methodNotAllowed.body();
+    expect(b1, 'Method not allowed');
+    final b2 = await Router.methodNotAllowed.body();
+    expect(b2, b1);
+  });
+
+  test('can call Router.methodNotAllowed.bytes() multiple times', () async {
+    final b1 = Router.methodNotAllowed.bytes();
+    expect(b1, emits(utf8.encode('Method not allowed')));
+    final b2 = Router.methodNotAllowed.bytes();
+    expect(b2, emits(utf8.encode('Method not allowed')));
+  });
+
+  test('can call Router.methodNotAllowed.copyWith()', () async {
+    final response = Router.methodNotAllowed.copyWith(headers: {'foo': 'bar'});
+    expect(response.headers['foo'], equals('bar'));
+    expect(response.body(), completion(equals('Method not allowed')));
+  });
+
   test('can mount route without params', () async {
     final context = _MockRequestContext();
     final app = Router()..mount('/', (RequestContext context) => Response());