From 28b6f84051693d449ab31a39273ea189847caaf5 Mon Sep 17 00:00:00 2001 From: Tobe O Date: Thu, 2 May 2019 18:37:32 -0400 Subject: [PATCH] Fix 404 hole --- CHANGELOG.md | 4 +++ lib/src/core/driver.dart | 8 +++-- pubspec.yaml | 2 +- test/404_hole_test.dart | 78 ++++++++++++++++++++++++++++++++++++++++ test/pretty_log.dart | 36 +++++++++++++++++++ 5 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 test/404_hole_test.dart create mode 100644 test/pretty_log.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index ad5a71a8..7801bfa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 2.0.1 +* Tracked down a bug in `Driver.runPipeline` that allowed fallback +handlers to run, even after the response was closed. + # 2.0.0 * Angel 2! :angel: :rocket: diff --git a/lib/src/core/driver.dart b/lib/src/core/driver.dart index fdb75c39..2be90faa 100644 --- a/lib/src/core/driver.dart +++ b/lib/src/core/driver.dart @@ -354,12 +354,16 @@ abstract class Driver< RequestContextType req, ResponseContextType res, Angel app) async { + var broken = false; while (it.moveNext()) { var current = it.current.handlers.iterator; - while (current.moveNext()) { + while (!broken && current.moveNext()) { var result = await app.executeHandler(current.current, req, res); - if (result != true) break; + if (result != true) { + broken = true; + break; + } } } } diff --git a/pubspec.yaml b/pubspec.yaml index 129e1a39..76da324c 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -29,6 +29,6 @@ dependencies: tuple: ^1.0.0 uuid: ^2.0.0-rc.1 dev_dependencies: - http: ^0.11.3 + http: ^0.12.0 io: ^0.3.0 test: ^1.0.0 diff --git a/test/404_hole_test.dart b/test/404_hole_test.dart new file mode 100644 index 00000000..c0b04a59 --- /dev/null +++ b/test/404_hole_test.dart @@ -0,0 +1,78 @@ +import 'dart:async'; +import 'package:angel_framework/angel_framework.dart'; +import 'package:angel_framework/http.dart'; +import 'package:charcode/ascii.dart'; +import 'package:http/io_client.dart' as http; +import 'package:logging/logging.dart'; +import 'package:test/test.dart'; +import 'pretty_log.dart'; + +void main() { + http.IOClient client; + AngelHttp driver; + Logger logger; + + setUp(() async { + client = http.IOClient(); + hierarchicalLoggingEnabled = true; + + logger = Logger.detached('404_hole') + ..level = Level.ALL + ..onRecord.listen(prettyLog); + + var app = Angel(logger: logger); + + app.fallback(hello); + app.fallback(throw404); + + // The error handler in the boilerplate. + var oldErrorHandler = app.errorHandler; + app.errorHandler = (e, req, res) async { + if (req.accepts('text/html', strict: true)) { + if (e.statusCode == 404 && req.accepts('text/html', strict: true)) { + await res + .render('error', {'message': 'No file exists at ${req.uri}.'}); + } else { + await res.render('error', {'message': e.message}); + } + } else { + return await oldErrorHandler(e, req, res); + } + }; + + driver = AngelHttp(app); + await driver.startServer(); + }); + + tearDown(() { + logger.clearListeners(); + client.close(); + scheduleMicrotask(driver.close); + }); + + test('does not continue processing after streaming', () async { + var url = driver.uri.replace(path: '/hey'); + for (int i = 0; i < 100; i++) { + var r = await client.get(url); + print('#$i: ${r.statusCode}: ${r.body}'); + expect(r.statusCode, 200); + expect(r.body, 'Hello!'); + } + }); +} + +/// Simulate angel_static +Future hello(RequestContext req, ResponseContext res) { + if (req.path == 'hey') { + var bytes = [$H, $e, $l, $l, $o, $exclamation]; + var s = Stream>.fromIterable([bytes]); + return s.pipe(res); + } else { + return Future.value(true); + } +} + +/// 404 +void throw404(RequestContext req, ResponseContext res) { + throw AngelHttpException.notFound(); +} diff --git a/test/pretty_log.dart b/test/pretty_log.dart new file mode 100644 index 00000000..9c03c110 --- /dev/null +++ b/test/pretty_log.dart @@ -0,0 +1,36 @@ +import 'package:logging/logging.dart'; +import 'package:io/ansi.dart'; + +/// Prints the contents of a [LogRecord] with pretty colors. +void prettyLog(LogRecord record) { + var code = chooseLogColor(record.level); + + if (record.error == null) print(code.wrap(record.toString())); + + if (record.error != null) { + var err = record.error; + print(code.wrap(record.toString() + '\n')); + print(code.wrap(err.toString())); + + if (record.stackTrace != null) { + print(code.wrap(record.stackTrace.toString())); + } + } +} + +/// Chooses a color based on the logger [level]. +AnsiCode chooseLogColor(Level level) { + if (level == Level.SHOUT) + return backgroundRed; + else if (level == Level.SEVERE) + return red; + else if (level == Level.WARNING) + return yellow; + else if (level == Level.INFO) + return cyan; + else if (level == Level.CONFIG || + level == Level.FINE || + level == Level.FINER || + level == Level.FINEST) return lightGray; + return resetAll; +}