timeouts, test for large files

sigurdm · sigurdm · commit 7113f81a9efe · 2025-10-20T10:34:19.000Z
diff --git a/pkg/image_proxy/lib/image_proxy_service.dart b/pkg/image_proxy/lib/image_proxy_service.dart
@@ -6,24 +6,20 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 import 'dart:typed_data';
+
 import 'package:crypto/crypto.dart';
+import 'package:googleapis/cloudkms/v1.dart' as kms;
+import 'package:googleapis_auth/auth_io.dart' as auth;
 import 'package:googleapis_auth/googleapis_auth.dart';
 import 'package:http/http.dart' as http;
-
 import 'package:retry/retry.dart';
 import 'package:shelf/shelf.dart' as shelf;
 import 'package:shelf/shelf_io.dart';
-import 'package:googleapis/cloudkms/v1.dart' as kms;
-import 'package:googleapis_auth/auth_io.dart' as auth;
-
-class Config {
-  final Uri secretsUrl;
-  final int maxFileSize;
-  Config({required this.secretsUrl, required this.maxFileSize});
-}
 
 bool isTesting = Platform.environment['IMAGE_PROXY_TESTING'] == 'true';
 
+Duration timeoutDelay = Duration(seconds: isTesting ? 1 : 8);
+
 /// The keys we currently allow the url to be signed with.
 Map<int, Uint8List> allowedKeys = {};
 
@@ -98,7 +94,10 @@ bool _constantTimeEquals(Uint8List a, Uint8List b) {
 
 // The client used for requesting the images.
 // Using raw dart:io client such that we can disable autoUncompress.
-final HttpClient client = HttpClient()..autoUncompress = false;
+final HttpClient client = HttpClient()
+  ..autoUncompress = false
+  ..connectionTimeout = Duration(seconds: 10)
+  ..idleTimeout = Duration(seconds: 15);
 
 final maxImageSize = 1024 * 1024 * 10; // At most 10 MB.
 
@@ -162,12 +161,21 @@ Future<shelf.Response> handler(shelf.Request request) async {
       String? contentEncoding;
       try {
         (statusCode, bytes, contentType, contentEncoding) = await retry(
-          maxDelay: isTesting ? Duration(seconds: 1) : Duration(seconds: 8),
+          maxDelay: timeoutDelay,
           maxAttempts: isTesting ? 2 : 8,
           () async {
             final request = await client.getUrl(parsedImageUrl);
+            Timer? timeoutTimer;
+            void scheduleRequestTimeout() {
+              timeoutTimer?.cancel();
+              timeoutTimer = Timer(timeoutDelay, () {
+                request.abort(RequestTimeoutException('No response'));
+              });
+            }
+
             request.headers.add('user-agent', 'pub-proxy');
             request.followRedirects = false;
+            scheduleRequestTimeout();
             var response = await request.close();
             var redirectCount = 0;
             while (response.isRedirect) {
@@ -184,9 +192,11 @@ Future<shelf.Response> handler(shelf.Request request) async {
               }
               final uri = parsedImageUrl.resolve(location);
               final request = await client.getUrl(uri);
+
               request.headers.add('user-agent', 'pub-proxy');
               // Set the body or headers as desired.
               request.followRedirects = false;
+              scheduleRequestTimeout();
               response = await request.close();
             }
             switch (response.statusCode) {
@@ -204,6 +214,11 @@ Future<shelf.Response> handler(shelf.Request request) async {
               await readAllBytes(
                 response,
                 contentLength == -1 ? maxImageSize : contentLength,
+              ).timeout(
+                timeoutDelay,
+                onTimeout: () {
+                  throw RequestTimeoutException('No response');
+                },
               ),
               response.headers.value('content-type'),
               response.headers.value('content-encoding'),
@@ -218,6 +233,8 @@ Future<shelf.Response> handler(shelf.Request request) async {
         return shelf.Response.badRequest(body: 'Image too large');
       } on RedirectException catch (e) {
         return shelf.Response.badRequest(body: e.message);
+      } on RequestTimeoutException catch (e) {
+        return shelf.Response.badRequest(body: e.message);
       } on ServerSideException catch (e) {
         return shelf.Response.badRequest(
           body: 'Failed to retrieve image. Status code ${e.statusCode}',
@@ -267,6 +284,11 @@ class RedirectException implements Exception {
   RedirectException(this.message);
 }
 
+class RequestTimeoutException implements Exception {
+  String message;
+  RequestTimeoutException(this.message);
+}
+
 Uint8List hmacSign(Uint8List key, Uint8List imageUrlBytes) {
   return Hmac(sha256, key).convert(imageUrlBytes).bytes as Uint8List;
 }
diff --git a/pkg/image_proxy/test/image_proxy_test.dart b/pkg/image_proxy/test/image_proxy_test.dart
@@ -2,6 +2,7 @@
 // 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 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
@@ -28,10 +29,16 @@ Future<int> startImageProxy() async {
   return port!;
 }
 
+Stream<List<int>> infiniteStream() async* {
+  while (true) {
+    yield List.generate(1000, (i) => 0);
+  }
+}
+
 Future<int> startImageServer() async {
   var i = 0;
   final server = await shelf_io.serve(
-    (shelf.Request request) {
+    (shelf.Request request) async {
       switch (request.url.path) {
         case 'path/to/image.jpg':
           return shelf.Response.ok(
@@ -73,6 +80,42 @@ Future<int> startImageServer() async {
             File(jpgImagePath).readAsBytesSync(),
             headers: {'content-type': 'image/jpeg'},
           );
+        case 'timeout':
+          await Future.delayed(Duration(hours: 1));
+          return shelf.Response.notFound('Not found');
+        case 'timeoutstreaming':
+          late final StreamController lateStreamController;
+          lateStreamController = StreamController(
+            onListen: () async {
+              // Return a single byte, and then stall.
+              lateStreamController.add([1]);
+              await Future.delayed(Duration(hours: 1));
+              await lateStreamController.close();
+            },
+          );
+          return shelf.Response.ok(
+            lateStreamController.stream,
+            headers: {'content-type': 'image/jpeg'},
+          );
+        case 'okstreaming':
+          return shelf.Response.ok(
+            // Has no content-length
+            File(jpgImagePath).openRead(),
+            headers: {'content-type': 'image/jpeg'},
+          );
+        case 'toobig':
+          return shelf.Response.ok(
+            infiniteStream(),
+            headers: {
+              'content-type': 'image/jpeg',
+              'content-length': '100000000',
+            },
+          );
+        case 'toobigstreaming':
+          return shelf.Response.ok(
+            infiniteStream(),
+            headers: {'content-type': 'image/jpeg'},
+          );
         default:
           return shelf.Response.notFound('Not found');
       }
@@ -168,6 +211,23 @@ Future<void> main() async {
       final expected = sha256.convert(File(svgImagePath).readAsBytesSync());
       expect(hash, expected);
     }
+
+    {
+      final response = await getImage(
+        day: today,
+        imageProxyPort: imageProxyPort,
+        imageServerPort: imageServerPort,
+        // Gives no content-length
+        pathToImage: 'okstreaming',
+      );
+      expect(response.statusCode, 200);
+      expect(response.headers['content-type']!.single, 'image/jpeg');
+      final jpgFile = File(jpgImagePath).readAsBytesSync();
+      expect(response.contentLength, jpgFile.length);
+      final hash = await sha256.bind(response).single;
+      final expected = sha256.convert(jpgFile);
+      expect(hash, expected);
+    }
   });
 
   test('Fails on days outside recent range', () async {
@@ -327,4 +387,62 @@ Future<void> main() async {
       expect(hash, expected);
     }
   });
+
+  test('times out', () async {
+    final imageProxyPort = await startImageProxy();
+    final imageServerPort = await startImageServer();
+    {
+      final response = await getImage(
+        imageProxyPort: imageProxyPort,
+        imageServerPort: imageServerPort,
+        day: today,
+        pathToImage: 'timeout',
+      );
+
+      expect(response.statusCode, 400);
+      // The proxy doesn't cache as long time as the original.
+      expect(await Utf8Codec().decodeStream(response), 'No response');
+    }
+    {
+      final response = await getImage(
+        imageProxyPort: imageProxyPort,
+        imageServerPort: imageServerPort,
+        day: today,
+        pathToImage: 'timeoutstreaming',
+      );
+
+      expect(response.statusCode, 400);
+      // The proxy doesn't cache as long time as the original.
+      expect(await Utf8Codec().decodeStream(response), 'No response');
+    }
+  });
+
+  test('protects against too big files', () async {
+    final imageProxyPort = await startImageProxy();
+    final imageServerPort = await startImageServer();
+    {
+      final response = await getImage(
+        imageProxyPort: imageProxyPort,
+        imageServerPort: imageServerPort,
+        day: today,
+        pathToImage: 'toobig',
+      );
+
+      expect(response.statusCode, 400);
+      // The proxy doesn't cache as long time as the original.
+      expect(await Utf8Codec().decodeStream(response), 'Image too large');
+    }
+    {
+      final response = await getImage(
+        imageProxyPort: imageProxyPort,
+        imageServerPort: imageServerPort,
+        day: today,
+        pathToImage: 'toobigstreaming',
+      );
+
+      expect(response.statusCode, 400);
+      // The proxy doesn't cache as long time as the original.
+      expect(await Utf8Codec().decodeStream(response), 'Image too large');
+    }
+  });
 }
