fix: 302 & 307 handling.

Author: llfbandit

http_cache_core/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 1.1.1
 - fix: Send only one condition on request cache validation.
 - fix: BaseRequest now has `headers` getter instead of `headerValuesAsList`.
+- fix: 302 & 307 codes handling.
 
 ## 1.1.0
 - feat: Early skip strategy calculation when request has already cache check conditions.

http_cache_core/lib/src/model/cache/cache_strategy.dart

@@ -143,15 +143,13 @@ class CacheStrategyFactory {
       return false;
     }
 
-    if (!allowedStatusCodes.contains(statusCode)) {
-      if (statusCode == 302 || statusCode == 307) {
-        // 302 & 307 can only be cached with the right response headers.
-        // https://datatracker.ietf.org/doc/html/rfc7234#section-3
-        if (response.headers[expiresHeader] == null &&
-            respCacheCtrl.maxAge == -1 &&
-            respCacheCtrl.privacy != null) {
-          return false;
-        }
+    if (statusCode == 302 || statusCode == 307) {
+      // 302 & 307 can only be cached with the right response headers.
+      // https://datatracker.ietf.org/doc/html/rfc7234#section-3
+      if (response.headers[expiresHeader] == null &&
+          respCacheCtrl.maxAge == -1 &&
+          respCacheCtrl.privacy == null) {
+        return false;
       }
     }
 

http_cache_core/lib/src/store/cache_store.dart

@@ -30,6 +30,7 @@ abstract class CacheStore {
   Future<void> set(CacheResponse response);
 
   /// Removes the given key from store.
+  ///
   /// [staleOnly] flag will remove it only if the key is expired
   /// (from maxStale).
   Future<void> delete(String key, {bool staleOnly = false});
@@ -52,7 +53,9 @@ abstract class CacheStore {
   });
 
   /// Removes all keys from store.
+  ///
   /// [priorityOrBelow] flag will remove keys only for the priority or below.
+  ///
   /// [staleOnly] flag will remove keys only if expired
   /// (from maxStale).
   ///

http_cache_core/test/cache_strategy_test.dart

@@ -274,6 +274,79 @@ void main() {
       expect(strategy.request!.headers[ifModifiedSinceHeader], isNotNull);
     });
 
+    test('compute returns cached response when cache is valid on 302 or 307',
+        () async {
+      Future<CacheStrategy> computeStrategy(
+        int statusCode,
+        Map<String, List<String>>? headers,
+      ) {
+        final request = MockRequest(
+          url: Uri.parse('https://ok.org'),
+          headers: {etagHeader: '1234'},
+        );
+        final response = MockResponse(statusCode: statusCode, headers: headers);
+
+        final factory = CacheStrategyFactory(
+          request: request,
+          cacheOptions: cacheOptions,
+          response: response,
+        );
+
+        return factory.compute(
+          cacheResponseBuilder: () async =>
+              cacheResponsefrom(cacheOptions, request, response),
+        );
+      }
+
+      Future<void> testStatusCode(int statusCode) async {
+        // public
+        var strategy = await computeStrategy(statusCode, {
+          'cache-control': ['public'],
+          etagHeader: ['1234'],
+        });
+        expect(strategy.cacheResponse, isNotNull);
+
+        // max-age
+        strategy = await computeStrategy(statusCode, {
+          'cache-control': ['max-age=3600'],
+          etagHeader: ['1234'],
+        });
+        expect(strategy.cacheResponse, isNotNull);
+
+        // expires
+        strategy = await computeStrategy(statusCode, {
+          expiresHeader: [HttpDate.format(DateTime.now())],
+          etagHeader: ['1234'],
+        });
+        expect(strategy.cacheResponse, isNotNull);
+
+        // public & max-age
+        strategy = await computeStrategy(statusCode, {
+          'cache-control': ['public, max-age=3600'],
+          etagHeader: ['1234'],
+        });
+        expect(strategy.cacheResponse, isNotNull);
+
+        // public & max-age & expires
+        strategy = await computeStrategy(statusCode, {
+          'cache-control': ['public, max-age=3600'],
+          expiresHeader: [HttpDate.format(DateTime.now())],
+          etagHeader: ['1234'],
+        });
+        expect(strategy.cacheResponse, isNotNull);
+
+        // no headers (default values)
+        strategy = await computeStrategy(statusCode, {
+          etagHeader: ['1234']
+        });
+        expect(strategy.request, isNotNull);
+        expect(strategy.cacheResponse, isNull);
+      }
+
+      await testStatusCode(302);
+      await testStatusCode(307);
+    });
+
     Future<BaseRequest> testWithPreconditionRequest(
       Map<String, String> headers, {
       String? eTag,

http_cache_file_store/test/file_store_test.dart

@@ -1,14 +1,17 @@
 import 'dart:io';
 
+import 'package:http_cache_core/http_cache_core.dart';
 import 'package:http_cache_file_store/http_cache_file_store.dart';
 import 'package:http_cache_store_tester/common_store_testing.dart';
+import 'package:path/path.dart' as path;
 import 'package:test/test.dart';
 
 void main() {
   late FileCacheStore store;
+  final dirPath = '${Directory.current.path}/test/data/file_store';
 
   setUpAll(() {
-    store = FileCacheStore('${Directory.current.path}/test/data/file_store');
+    store = FileCacheStore(dirPath);
   });
 
   setUp(() async {
@@ -29,6 +32,27 @@ void main() {
   test('pathExists', () => pathExists(store));
   test('deleteFromPath', () => deleteFromPath(store));
   test('getFromPath', () => getFromPath(store));
+
+  test(
+    'Corrupted file',
+    () async {
+      await addFooResponse(store, key: 'corrupt');
+      expect(await store.get('corrupt'), isNotNull);
+
+      // corrupt file
+      final file =
+          File(path.join(dirPath, CachePriority.normal.name, 'corrupt'));
+      if (!file.existsSync()) {
+        throw Exception('Unexpected missing file.');
+      }
+
+      final bytes = file.readAsBytesSync();
+      await file.writeAsBytes(bytes.sublist(0, bytes.length ~/ 2));
+
+      expect(await store.get('corrupt'), isNull);
+    },
+  );
+
   test(
     'Concurrent access',
     () async => await concurrentAccess(store),

http_cache_store_tester/lib/common_store_testing.dart

@@ -4,7 +4,7 @@ import 'dart:convert';
 import 'package:http_cache_core/http_cache_core.dart';
 import 'package:test/test.dart';
 
-Future<void> _addFooResponse(
+Future<void> addFooResponse(
   CacheStore store, {
   String key = 'foo',
   CacheControl? cacheControl,
@@ -41,8 +41,17 @@ Future<void> emptyByDefault(CacheStore store) async {
 }
 
 Future<void> addItem(CacheStore store) async {
-  await _addFooResponse(store);
-  expect(await store.exists('foo'), isTrue);
+  await addFooResponse(store, priority: CachePriority.low);
+  var resp = await store.get('foo');
+  expect(resp?.priority, CachePriority.low);
+
+  await addFooResponse(store, priority: CachePriority.normal);
+  resp = await store.get('foo');
+  expect(resp?.priority, CachePriority.normal);
+
+  await addFooResponse(store, priority: CachePriority.high);
+  resp = await store.get('foo');
+  expect(resp?.priority, CachePriority.high);
 }
 
 Future<void> getItem(CacheStore store) async {
@@ -53,7 +62,7 @@ Future<void> getItem(CacheStore store) async {
   final expires = DateTime.now();
   final lastModified = HttpDate.format(DateTime.now());
 
-  await _addFooResponse(
+  await addFooResponse(
     store,
     maxStale: DateTime.now().add(const Duration(days: 1)),
     headers: headers,
@@ -80,14 +89,14 @@ Future<void> getItem(CacheStore store) async {
 }
 
 Future<void> deleteItem(CacheStore store) async {
-  await _addFooResponse(store);
+  await addFooResponse(store);
   expect(await store.exists('foo'), isTrue);
 
   await store.delete('foo');
   expect(await store.exists('foo'), isFalse);
   await store.delete('foo'); // check for non exception
 
-  await _addFooResponse(
+  await addFooResponse(
     store,
     maxStale: DateTime.now().add(const Duration(days: 1)),
   );
@@ -98,12 +107,12 @@ Future<void> deleteItem(CacheStore store) async {
 }
 
 Future<void> clean(CacheStore store) async {
-  await _addFooResponse(
+  await addFooResponse(
     store,
     key: 'not-stale',
     maxStale: DateTime.now().add(const Duration(days: 1)),
   );
-  await _addFooResponse(
+  await addFooResponse(
     store,
     key: 'stale',
     maxStale: DateTime.now().subtract(const Duration(days: 1)),
@@ -127,7 +136,7 @@ Future<void> clean(CacheStore store) async {
 
 Future<void> expires(CacheStore store) async {
   final now = DateTime.now();
-  await _addFooResponse(store, expires: DateTime.now());
+  await addFooResponse(store, expires: DateTime.now());
   final resp = await store.get('foo');
   expect(
     resp!.expires!.subtract(
@@ -146,7 +155,7 @@ Future<void> expires(CacheStore store) async {
 Future<void> lastModified(CacheStore store) async {
   final lastModified = 'Wed, 21 Oct 2015 07:28:00 GMT';
 
-  await _addFooResponse(store, lastModified: lastModified);
+  await addFooResponse(store, lastModified: lastModified);
   final resp = await store.get('foo');
   expect(resp!.lastModified, equals(lastModified));
 }
@@ -159,7 +168,7 @@ Future<void> concurrentAccess(CacheStore store) async {
 
   for (var i = 1; i <= max; i++) {
     final key = i % 3 == 0 ? 'bar' : 'foo';
-    _addFooResponse(store, key: key, lastModified: lastModified).then(
+    addFooResponse(store, key: key, lastModified: lastModified).then(
       (value) {
         store.get(key).then(
           (resp) {
@@ -265,28 +274,28 @@ void pathExists(CacheStore store) {
 }
 
 Future<void> deleteFromPath(CacheStore store) async {
-  await _addFooResponse(store);
+  await addFooResponse(store);
   expect(await store.exists('foo'), isTrue);
   await store.deleteFromPath(RegExp('https://foo.com'));
   expect(await store.exists('foo'), isFalse);
 
-  await _addFooResponse(store, url: 'https://foo.com?bar=bar');
+  await addFooResponse(store, url: 'https://foo.com?bar=bar');
   expect(await store.exists('foo'), isTrue);
   await store.deleteFromPath(
     RegExp('https://foo.com'),
     queryParams: {'bar': null},
   );
   expect(await store.exists('foo'), isFalse);
 
-  await _addFooResponse(store, url: 'https://foo.com?bar=bar');
+  await addFooResponse(store, url: 'https://foo.com?bar=bar');
   expect(await store.exists('foo'), isTrue);
   await store.deleteFromPath(
     RegExp('https://foo.com'),
     queryParams: {'bar': 'bar'},
   );
   expect(await store.exists('foo'), isFalse);
 
-  await _addFooResponse(store, url: 'https://foo.com?bar=bar');
+  await addFooResponse(store, url: 'https://foo.com?bar=bar');
   expect(await store.exists('foo'), isTrue);
   await store.deleteFromPath(
     RegExp('https://foo.com'),
@@ -296,25 +305,25 @@ Future<void> deleteFromPath(CacheStore store) async {
 }
 
 Future<void> getFromPath(CacheStore store) async {
-  await _addFooResponse(store);
+  await addFooResponse(store);
   var list = await store.getFromPath(RegExp('https://foo.com'));
   expect(list.length, 1);
 
-  await _addFooResponse(store, url: 'https://foo.com?bar=bar');
+  await addFooResponse(store, url: 'https://foo.com?bar=bar');
   list = await store.getFromPath(
     RegExp('https://foo.com'),
     queryParams: {'bar': null},
   );
   expect(list.length, 1);
 
-  await _addFooResponse(store, url: 'https://foo.com?bar=bar');
+  await addFooResponse(store, url: 'https://foo.com?bar=bar');
   list = await store.getFromPath(
     RegExp('https://foo.com'),
     queryParams: {'bar': 'bar'},
   );
   expect(list.length, 1);
 
-  await _addFooResponse(store, url: 'https://foo.com?bar=bar');
+  await addFooResponse(store, url: 'https://foo.com?bar=bar');
   list = await store.getFromPath(
     RegExp('https://foo.com'),
     queryParams: {'bar': 'foobar'},