From 8d14ad75f96987d9a92142942f9375c96e9e3974 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Fri, 7 May 2021 16:07:20 -0700 Subject: [PATCH] Add an option to only send OK (200) responses for GET and HEAD requests Fixes https://github.com/dart-lang/shelf_static/issues/53 Also prepare to release v1.1.0 --- CHANGELOG.md | 3 ++ lib/shelf_static.dart | 2 +- lib/src/static_handler.dart | 99 ++++++++++++++++++++++++------------- lib/src/util.dart | 7 +++ pubspec.yaml | 2 +- test/basic_file_test.dart | 41 +++++++++++++++ test/test_util.dart | 2 +- 7 files changed, 120 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60be005..5000bbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## 1.1.0-dev +* Added optional `onlyGetMethods` parameter to `createStaticHandler` and + `createFileHandler`. When `true`, only sends an OK (200) response when the + HTTP request method is "GET" or "HEAD". * Correctly handle `HEAD` requests. ## 1.0.0 diff --git a/lib/shelf_static.dart b/lib/shelf_static.dart index 0d95129..53910a1 100644 --- a/lib/shelf_static.dart +++ b/lib/shelf_static.dart @@ -2,4 +2,4 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -export 'src/static_handler.dart'; +export 'src/static_handler.dart' show createFileHandler, createStaticHandler; diff --git a/lib/src/static_handler.dart b/lib/src/static_handler.dart index 11220aa..19d68d2 100644 --- a/lib/src/static_handler.dart +++ b/lib/src/static_handler.dart @@ -39,12 +39,18 @@ final _defaultMimeTypeResolver = MimeTypeResolver(); /// /// Specify a custom [contentTypeResolver] to customize automatic content type /// detection. -Handler createStaticHandler(String fileSystemPath, - {bool serveFilesOutsidePath = false, - String? defaultDocument, - bool listDirectories = false, - bool useHeaderBytesForContentType = false, - MimeTypeResolver? contentTypeResolver}) { +/// +/// If [onlyGetMethods] is `true`, then OK (200) responses are only sent for GET +/// and HEAD requests. Others receive a "not found" (404). +Handler createStaticHandler( + String fileSystemPath, { + bool serveFilesOutsidePath = false, + String? defaultDocument, + bool listDirectories = false, + bool useHeaderBytesForContentType = false, + MimeTypeResolver? contentTypeResolver, + bool onlyGetMethods = false, +}) { final rootDir = Directory(fileSystemPath); if (!rootDir.existsSync()) { throw ArgumentError('A directory corresponding to fileSystemPath ' @@ -82,7 +88,7 @@ Handler createStaticHandler(String fileSystemPath, } if (fileFound == null) { - return Response.notFound('Not Found'); + return notFound(); } final file = fileFound; @@ -91,7 +97,7 @@ Handler createStaticHandler(String fileSystemPath, // Do not serve a file outside of the original fileSystemPath if (!p.isWithin(fileSystemPath, resolvedPath)) { - return Response.notFound('Not Found'); + return notFound(); } } @@ -103,31 +109,37 @@ Handler createStaticHandler(String fileSystemPath, return _redirectToAddTrailingSlash(uri); } - return _handleFile(request, file, () async { - if (useHeaderBytesForContentType) { - final length = - math.min(mimeResolver.magicNumbersMaxLength, file.lengthSync()); - - final byteSink = ByteAccumulatorSink(); - - await file.openRead(0, length).listen(byteSink.add).asFuture(); - - return mimeResolver.lookup(file.path, headerBytes: byteSink.bytes); - } else { - return mimeResolver.lookup(file.path); - } - }); + return _handleFile( + request, + file, + () async { + if (useHeaderBytesForContentType) { + final length = + math.min(mimeResolver.magicNumbersMaxLength, file.lengthSync()); + + final byteSink = ByteAccumulatorSink(); + + await file.openRead(0, length).listen(byteSink.add).asFuture(); + + return mimeResolver.lookup(file.path, headerBytes: byteSink.bytes); + } else { + return mimeResolver.lookup(file.path); + } + }, + onlyGetMethods: onlyGetMethods, + ); }; } Response _redirectToAddTrailingSlash(Uri uri) { final location = Uri( - scheme: uri.scheme, - userInfo: uri.userInfo, - host: uri.host, - port: uri.port, - path: '${uri.path}/', - query: uri.query); + scheme: uri.scheme, + userInfo: uri.userInfo, + host: uri.host, + port: uri.port, + path: '${uri.path}/', + query: uri.query, + ); return Response.movedPermanently(location.toString()); } @@ -154,7 +166,15 @@ File? _tryDefaultFile(String dirPath, String? defaultFile) { /// This uses the given [contentType] for the Content-Type header. It defaults /// to looking up a content type based on [path]'s file extension, and failing /// that doesn't sent a [contentType] header at all. -Handler createFileHandler(String path, {String? url, String? contentType}) { +/// +/// If [onlyGetMethods] is `true`, then OK (200) responses are only sent for GET +/// and HEAD requests. Others receive a "not found" (404). +Handler createFileHandler( + String path, { + String? url, + String? contentType, + bool onlyGetMethods = false, +}) { final file = File(path); if (!file.existsSync()) { throw ArgumentError.value(path, 'path', 'does not exist.'); @@ -166,8 +186,13 @@ Handler createFileHandler(String path, {String? url, String? contentType}) { url ??= p.toUri(p.basename(path)).toString(); return (request) { - if (request.url.path != url) return Response.notFound('Not Found'); - return _handleFile(request, file, () => mimeType); + if (request.url.path != url) return notFound(); + return _handleFile( + request, + file, + () => mimeType, + onlyGetMethods: onlyGetMethods, + ); }; } @@ -176,8 +201,16 @@ Handler createFileHandler(String path, {String? url, String? contentType}) { /// This handles caching, and sends a 304 Not Modified response if the request /// indicates that it has the latest version of a file. Otherwise, it calls /// [getContentType] and uses it to populate the Content-Type header. -Future _handleFile(Request request, File file, - FutureOr Function() getContentType) async { +Future _handleFile( + Request request, + File file, + FutureOr Function() getContentType, { + required bool onlyGetMethods, +}) async { + if (onlyGetMethods && !getMethods.contains(request.method)) { + return notFound(); + } + final stat = file.statSync(); final ifModifiedSince = request.ifModifiedSince; diff --git a/lib/src/util.dart b/lib/src/util.dart index 2d42379..5bcf42f 100644 --- a/lib/src/util.dart +++ b/lib/src/util.dart @@ -2,7 +2,14 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'package:shelf/shelf.dart'; + DateTime toSecondResolution(DateTime dt) { if (dt.millisecond == 0) return dt; return dt.subtract(Duration(milliseconds: dt.millisecond)); } + +Response notFound() => Response.notFound('Not Found'); + +/// HTTP methods which return OK (200) responses with static file servers. +const getMethods = {'GET', 'HEAD'}; diff --git a/pubspec.yaml b/pubspec.yaml index c9419c9..14dea56 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: shelf_static -version: 1.1.0-dev +version: 1.1.0 description: Static file server support for shelf repository: https://github.com/dart-lang/shelf_static diff --git a/test/basic_file_test.dart b/test/basic_file_test.dart index a1425da..44a7040 100644 --- a/test/basic_file_test.dart +++ b/test/basic_file_test.dart @@ -9,6 +9,7 @@ import 'package:http_parser/http_parser.dart'; import 'package:mime/mime.dart' as mime; import 'package:path/path.dart' as p; import 'package:shelf_static/shelf_static.dart'; +import 'package:shelf_static/src/util.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; @@ -233,4 +234,44 @@ void main() { expect(response.mimeType, 'image/webp'); }); }); + + group('HTTP method', () { + for (var method in _httpMethods) { + group(method, () { + test('with', () async { + final handler = createStaticHandler(d.sandbox); + + final response = + await makeRequest(handler, '/root.txt', method: method); + expect(response.statusCode, HttpStatus.ok); + expect(response.contentLength, 8); + expect( + await response.readAsString(), + method == 'HEAD' ? isEmpty : 'root txt', + ); + }); + + test('without', () async { + final handler = createStaticHandler(d.sandbox, onlyGetMethods: true); + + final response = + await makeRequest(handler, '/root.txt', method: method); + + if (getMethods.contains(method)) { + expect(response.statusCode, HttpStatus.ok); + expect(response.contentLength, 8); + expect( + await response.readAsString(), + method == 'HEAD' ? isEmpty : 'root txt', + ); + } else { + expect(response.statusCode, HttpStatus.notFound); + expect(await response.readAsString(), 'Not Found'); + } + }); + }); + } + }); } + +const _httpMethods = {'PUT', 'POST', 'DELETE', 'PATCH', ...getMethods}; diff --git a/test/test_util.dart b/test/test_util.dart index 5c628d8..dada6c1 100644 --- a/test/test_util.dart +++ b/test/test_util.dart @@ -35,7 +35,7 @@ Handler _rootHandler(String? path, Handler handler) { return (Request request) { if (!_ctx.isWithin('/$path', request.requestedUri.path)) { - return Response.notFound('not found'); + return notFound(); } assert(request.handlerPath == '/');