Refactor range support implementation (#66)
Reduce nesting by extracting to a method allowing guard clauses and
inverted the conditionals with early returns. Remove some comments that
repeat the code. Use conditional map elements and `..[]` operator to
reduce the noise around setting headers.
Add tests for requests where the range header is ignored.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a885d2a..8be7a56 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,5 @@
+## 1.1.1-dev
+
## 1.1.0
* Correctly handle `HEAD` requests.
diff --git a/lib/src/static_handler.dart b/lib/src/static_handler.dart
index c290d95..463c731 100644
--- a/lib/src/static_handler.dart
+++ b/lib/src/static_handler.dart
@@ -188,74 +188,71 @@
}
}
+ final contentType = await getContentType();
final headers = {
HttpHeaders.lastModifiedHeader: formatHttpDate(stat.modified),
HttpHeaders.acceptRangesHeader: 'bytes',
+ if (contentType != null) HttpHeaders.contentTypeHeader: contentType,
};
- final contentType = await getContentType();
- final length = await file.length();
+ return _fileRangeResponse(request, file, headers) ??
+ Response.ok(
+ request.method == 'HEAD' ? null : file.openRead(),
+ headers: headers..[HttpHeaders.contentLengthHeader] = '${stat.size}',
+ );
+}
+
+/// Serves a range of [file], if [request] is valid 'bytes' range request.
+///
+/// If the request does not specify a range, specifies a range of the wrong
+/// type, or has a syntactic error the range is ignored and `null` is returned.
+///
+/// If the range request is valid but the file is not long enough to include the
+/// start of the range a range not satisfiable response is returned.
+///
+/// Ranges that end past the end of the file are truncated.
+Response? _fileRangeResponse(
+ Request request, File file, Map<String, Object> headers) {
final range = request.headers[HttpHeaders.rangeHeader];
- if (contentType != null) headers[HttpHeaders.contentTypeHeader] = contentType;
- if (range != null) {
- // We only support one range, where the standard support several.
- final matches = RegExp(r'^bytes=(\d*)\-(\d*)$').firstMatch(range);
- // If the range header have the right format, handle it.
- if (matches != null) {
- final startMatch = matches[1]!;
- final endMatch = matches[2]!;
- if (startMatch.isNotEmpty || endMatch.isNotEmpty) {
- // Serve sub-range.
- int start; // First byte position - inclusive.
- int end; // Last byte position - inclusive.
- if (startMatch.isEmpty) {
- start = length - int.parse(endMatch);
- if (start < 0) start = 0;
- end = length - 1;
- } else {
- start = int.parse(startMatch);
- end = endMatch.isEmpty ? length - 1 : int.parse(endMatch);
- }
- // If the range is syntactically invalid the Range header
- // MUST be ignored (RFC 2616 section 14.35.1).
- if (start <= end) {
- if (end >= length) {
- end = length - 1;
- }
- if (start >= length) {
- return Response(
- HttpStatus.requestedRangeNotSatisfiable,
- headers: headers,
- );
- }
+ if (range == null) return null;
+ final matches = RegExp(r'^bytes=(\d*)\-(\d*)$').firstMatch(range);
+ // Ignore ranges other than bytes
+ if (matches == null) return null;
- // Override Content-Length with the actual bytes sent.
- headers[HttpHeaders.contentLengthHeader] =
- (end - start + 1).toString();
+ final actualLength = file.lengthSync();
+ final startMatch = matches[1]!;
+ final endMatch = matches[2]!;
+ if (startMatch.isEmpty && endMatch.isEmpty) return null;
- // Set 'Partial Content' status code.
- headers[HttpHeaders.contentRangeHeader] = 'bytes $start-$end/$length';
- // Pipe the 'range' of the file.
- if (request.method == 'HEAD') {
- return Response(
- HttpStatus.partialContent,
- headers: headers,
- );
- } else {
- return Response(
- HttpStatus.partialContent,
- body: file.openRead(start, end + 1),
- headers: headers,
- );
- }
- }
- }
- }
+ int start; // First byte position - inclusive.
+ int end; // Last byte position - inclusive.
+ if (startMatch.isEmpty) {
+ start = actualLength - int.parse(endMatch);
+ if (start < 0) start = 0;
+ end = actualLength - 1;
+ } else {
+ start = int.parse(startMatch);
+ end = endMatch.isEmpty ? actualLength - 1 : int.parse(endMatch);
}
- headers[HttpHeaders.contentLengthHeader] = stat.size.toString();
- return Response.ok(
- request.method == 'HEAD' ? null : file.openRead(),
- headers: headers,
+ // If the range is syntactically invalid the Range header
+ // MUST be ignored (RFC 2616 section 14.35.1).
+ if (start > end) return null;
+
+ if (end >= actualLength) {
+ end = actualLength - 1;
+ }
+ if (start >= actualLength) {
+ return Response(
+ HttpStatus.requestedRangeNotSatisfiable,
+ headers: headers,
+ );
+ }
+ return Response(
+ HttpStatus.partialContent,
+ body: request.method == 'HEAD' ? null : file.openRead(start, end + 1),
+ headers: headers
+ ..[HttpHeaders.contentLengthHeader] = (end - start + 1).toString()
+ ..[HttpHeaders.contentRangeHeader] = 'bytes $start-$end/$actualLength',
);
}
diff --git a/pubspec.yaml b/pubspec.yaml
index 824b2d6..08d08dc 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: shelf_static
-version: 1.1.0
+version: 1.1.1-dev
description: Static file server support for the shelf package and ecosystem
repository: https://github.com/dart-lang/shelf_static
diff --git a/test/create_file_handler_test.dart b/test/create_file_handler_test.dart
index 907e3b4..91c4c86 100644
--- a/test/create_file_handler_test.dart
+++ b/test/create_file_handler_test.dart
@@ -90,6 +90,7 @@
);
expect(response.headers, containsPair('content-length', '5'));
});
+
test('at the end of has overflow from 0 to 9', () async {
final handler = createFileHandler(p.join(d.sandbox, 'file.txt'));
final response = await makeRequest(
@@ -111,6 +112,7 @@
);
expect(response.headers, containsPair('content-length', '8'));
});
+
test('at the start of has overflow from 8 to 9', () async {
final handler = createFileHandler(p.join(d.sandbox, 'file.txt'));
final response = await makeRequest(
@@ -128,6 +130,54 @@
HttpStatus.requestedRangeNotSatisfiable,
);
});
+
+ test('ignores invalid request with start > end', () async {
+ final handler = createFileHandler(p.join(d.sandbox, 'file.txt'));
+ final response = await makeRequest(
+ handler,
+ '/file.txt',
+ headers: {'range': 'bytes=2-1'},
+ );
+ expect(response.statusCode, equals(HttpStatus.ok));
+ expect(response.contentLength, equals(8));
+ expect(response.readAsString(), completion(equals('contents')));
+ });
+
+ test('ignores request with start > end', () async {
+ final handler = createFileHandler(p.join(d.sandbox, 'file.txt'));
+ final response = await makeRequest(
+ handler,
+ '/file.txt',
+ headers: {'range': 'bytes=2-1'},
+ );
+ expect(response.statusCode, equals(HttpStatus.ok));
+ expect(response.contentLength, equals(8));
+ expect(response.readAsString(), completion(equals('contents')));
+ });
+
+ test('ignores request with units other than bytes', () async {
+ final handler = createFileHandler(p.join(d.sandbox, 'file.txt'));
+ final response = await makeRequest(
+ handler,
+ '/file.txt',
+ headers: {'range': 'not-bytes=0-1'},
+ );
+ expect(response.statusCode, equals(HttpStatus.ok));
+ expect(response.contentLength, equals(8));
+ expect(response.readAsString(), completion(equals('contents')));
+ });
+
+ test('ignores request with no start or end', () async {
+ final handler = createFileHandler(p.join(d.sandbox, 'file.txt'));
+ final response = await makeRequest(
+ handler,
+ '/file.txt',
+ headers: {'range': 'bytes=-'},
+ );
+ expect(response.statusCode, equals(HttpStatus.ok));
+ expect(response.contentLength, equals(8));
+ expect(response.readAsString(), completion(equals('contents')));
+ });
});
group('throws an ArgumentError for', () {
