From 52ecd56174ece0cf77c76b33cde0b60c88c8badb Mon Sep 17 00:00:00 2001 From: Alex Li Date: Mon, 20 Jul 2026 23:51:15 +0800 Subject: [PATCH 1/2] fix(cookie_manager): prevent duplicate cookies on reused request options Track the source and generated Cookie headers per RequestOptions. Repeated interceptor passes now merge caller input with the latest cookie jar state. Preserve existing same-name caller cookies. Cover jar updates, deletion, origin changes, and isolated request state. Co-Authored-By: Codex --- plugins/cookie_manager/CHANGELOG.md | 3 +- .../cookie_manager/lib/src/cookie_mgr.dart | 19 +- plugins/cookie_manager/test/cookies_test.dart | 251 ++++++++++++++++-- 3 files changed, 253 insertions(+), 20 deletions(-) diff --git a/plugins/cookie_manager/CHANGELOG.md b/plugins/cookie_manager/CHANGELOG.md index 5c8f50138..1e81ef9ec 100644 --- a/plugins/cookie_manager/CHANGELOG.md +++ b/plugins/cookie_manager/CHANGELOG.md @@ -2,7 +2,8 @@ ## Unreleased -*None.* +- Prevent duplicate cookies when reusing request options while preserving + caller-provided cookies. ## 3.4.0 diff --git a/plugins/cookie_manager/lib/src/cookie_mgr.dart b/plugins/cookie_manager/lib/src/cookie_mgr.dart index a3a79d17a..6b29de3a7 100644 --- a/plugins/cookie_manager/lib/src/cookie_mgr.dart +++ b/plugins/cookie_manager/lib/src/cookie_mgr.dart @@ -21,6 +21,13 @@ const _kIsWeb = _kIsWebInterop || _kIsWebUtil || identical(0, 0.0); /// attribute like "expires=Sun, 19 Feb 3000 01:43:15 GMT", which could also contain commas. final _setCookieReg = RegExp('(?<=)(,)(?=[^;]+?=)'); +class _CookieHeaderState { + const _CookieHeaderState(this.source, this.merged); + + final String? source; + final String merged; +} + /// Cookie manager for HTTP requests based on [CookieJar]. class CookieManager extends Interceptor { CookieManager( @@ -38,6 +45,8 @@ class CookieManager extends Interceptor { /// Whether to ignore invalid cookies during parsing or saving. bool ignoreInvalidCookies; + final Expando<_CookieHeaderState> _cookieHeaderStates = Expando(); + /// Merge cookies into a Cookie string. /// Cookies with longer paths are listed before cookies with shorter paths. static String getCookies(List cookies) { @@ -149,14 +158,22 @@ class CookieManager extends Interceptor { final savedCookies = await cookieJar.loadForRequest(options.uri); final previousCookies = options.headers[HttpHeaders.cookieHeader] as String?; + final previousState = _cookieHeaderStates[options]; + // Rebuild a header that still exactly matches this manager's last output. + // Any other header becomes the source for the next merge. + final sourceCookies = + previousState != null && previousState.merged == previousCookies + ? previousState.source + : previousCookies; final cookies = getCookies([ - ...?previousCookies + ...?sourceCookies ?.split(';') .where((e) => e.isNotEmpty) .map((c) => _fromSetCookieValue(c)) .whereType(), // Use .nonNulls when the minimum SDK is 3.0. ...savedCookies, ]); + _cookieHeaderStates[options] = _CookieHeaderState(sourceCookies, cookies); return cookies; } diff --git a/plugins/cookie_manager/test/cookies_test.dart b/plugins/cookie_manager/test/cookies_test.dart index b01f8ea41..08dcf51c7 100644 --- a/plugins/cookie_manager/test/cookies_test.dart +++ b/plugins/cookie_manager/test/cookies_test.dart @@ -1,4 +1,5 @@ @TestOn('vm') +import 'dart:async'; import 'dart:io'; import 'dart:typed_data'; @@ -8,21 +9,39 @@ import 'package:dio/io.dart'; import 'package:dio_cookie_manager/dio_cookie_manager.dart'; import 'package:test/test.dart'; -class MockRequestInterceptorHandler extends RequestInterceptorHandler { - MockRequestInterceptorHandler(this.expectResult); +class _TestRequestInterceptorHandler extends RequestInterceptorHandler { + final Completer _result = Completer(); - final String expectResult; + Future get result => _result.future; @override void next(RequestOptions requestOptions) { - final c = requestOptions.headers[HttpHeaders.cookieHeader]; - expect(c == expectResult, true); - super.next(requestOptions); + _result.complete(requestOptions); + } + + @override + void reject( + DioException error, [ + bool callFollowingErrorInterceptor = false, + ]) { + _result.completeError(error, error.stackTrace); } } class MockResponseInterceptorHandler extends ResponseInterceptorHandler {} +Future expectRequestCookies( + CookieManager cookieManager, + RequestOptions options, + String? expected, +) async { + final handler = _TestRequestInterceptorHandler(); + await cookieManager.onRequest(options, handler); + final result = await handler.result; + expect(result, same(options)); + expect(options.headers[HttpHeaders.cookieHeader], expected); +} + class _MockRejectRequestInterceptorHandler extends RequestInterceptorHandler { _MockRejectRequestInterceptorHandler(this.matcher); @@ -118,18 +137,200 @@ void main() { ); // Verify mock cookies. - final mockRequestInterceptorHandler = - MockRequestInterceptorHandler(expectResult); final options = RequestOptions( baseUrl: exampleUrl, headers: { HttpHeaders.cookieHeader: mockSecondRequestCookies, }, ); - await cookieManager.onRequest( - options, - mockRequestInterceptorHandler, - ); + await expectRequestCookies(cookieManager, options, expectResult); + }); + + group('reusing request options', () { + const exampleUrl = 'https://example.com/api/endpoint'; + + test('does not append the cookie jar values again through Dio.fetch', + () async { + final cookieJar = CookieJar(); + await cookieJar.saveFromResponse( + Uri.parse(exampleUrl), + [ + Cookie('session', 'root')..path = '/', + Cookie('session', 'api')..path = '/api', + ], + ); + final cookieManager = CookieManager(cookieJar); + final adapter = _CookieRecordingAdapter(); + final dio = Dio() + ..httpClientAdapter = adapter + ..interceptors.add(cookieManager); + addTearDown(dio.close); + final options = RequestOptions( + baseUrl: exampleUrl, + responseType: ResponseType.plain, + ); + + await dio.fetch(options); + await dio.fetch(options); + await dio.fetch(options); + + expect( + adapter.cookieHeaders, + everyElement( + equals( + 'session=api; session=root', + ), + ), + ); + expect(adapter.cookieHeaders, hasLength(3)); + expect(adapter.requestOptions, everyElement(same(options))); + }); + + test('preserves a same-name cookie supplied by the caller', () async { + final cookieJar = CookieJar(); + await cookieJar.saveFromResponse( + Uri.parse(exampleUrl), + [Cookie('session', 'saved')..path = '/'], + ); + final cookieManager = CookieManager(cookieJar); + final options = RequestOptions( + baseUrl: exampleUrl, + headers: {HttpHeaders.cookieHeader: 'session=provided'}, + ); + + await expectRequestCookies( + cookieManager, + options, + 'session=provided; session=saved', + ); + await expectRequestCookies( + cookieManager, + options, + 'session=provided; session=saved', + ); + }); + + test('reloads saved cookies without retaining their old values', () async { + final cookieJar = CookieJar(); + final uri = Uri.parse(exampleUrl); + await cookieJar.saveFromResponse( + uri, + [Cookie('session', 'old')..path = '/'], + ); + final cookieManager = CookieManager(cookieJar); + final options = RequestOptions( + baseUrl: exampleUrl, + headers: {HttpHeaders.cookieHeader: 'provided=value'}, + ); + + await expectRequestCookies( + cookieManager, + options, + 'provided=value; session=old', + ); + await cookieJar.saveFromResponse( + uri, + [Cookie('session', 'new')..path = '/'], + ); + await expectRequestCookies( + cookieManager, + options, + 'provided=value; session=new', + ); + await cookieJar.deleteAll(); + await expectRequestCookies(cookieManager, options, 'provided=value'); + }); + + test('uses a cookie header changed by the caller as the new input', + () async { + final cookieJar = CookieJar(); + await cookieJar.saveFromResponse( + Uri.parse(exampleUrl), + [Cookie('saved', 'value')..path = '/'], + ); + final cookieManager = CookieManager(cookieJar); + final options = RequestOptions( + baseUrl: exampleUrl, + headers: {HttpHeaders.cookieHeader: 'provided=first'}, + ); + + await expectRequestCookies( + cookieManager, + options, + 'provided=first; saved=value', + ); + options.headers[HttpHeaders.cookieHeader] = 'provided=changed'; + await expectRequestCookies( + cookieManager, + options, + 'provided=changed; saved=value', + ); + await expectRequestCookies( + cookieManager, + options, + 'provided=changed; saved=value', + ); + }); + + test('does not retain saved cookies after the request origin changes', + () async { + final cookieJar = CookieJar(); + await cookieJar.saveFromResponse( + Uri.parse(exampleUrl), + [Cookie('session', 'saved')..path = '/'], + ); + final cookieManager = CookieManager(cookieJar); + final options = RequestOptions( + baseUrl: exampleUrl, + headers: {HttpHeaders.cookieHeader: 'provided=value'}, + ); + + await expectRequestCookies( + cookieManager, + options, + 'provided=value; session=saved', + ); + options.baseUrl = 'https://other.example.com'; + await expectRequestCookies(cookieManager, options, 'provided=value'); + }); + + test('keeps state separate between request options', () async { + final cookieJar = CookieJar(); + await cookieJar.saveFromResponse( + Uri.parse(exampleUrl), + [Cookie('saved', 'value')..path = '/'], + ); + final cookieManager = CookieManager(cookieJar); + final first = RequestOptions( + baseUrl: exampleUrl, + headers: {HttpHeaders.cookieHeader: 'provided=first'}, + ); + final second = RequestOptions( + baseUrl: exampleUrl, + headers: {HttpHeaders.cookieHeader: 'provided=second'}, + ); + + await expectRequestCookies( + cookieManager, + first, + 'provided=first; saved=value', + ); + await expectRequestCookies( + cookieManager, + second, + 'provided=second; saved=value', + ); + await expectRequestCookies( + cookieManager, + first, + 'provided=first; saved=value', + ); + await expectRequestCookies( + cookieManager, + second, + 'provided=second; saved=value', + ); + }); }); group('Set-Cookie', () { @@ -162,12 +363,7 @@ void main() { // Verify mock cookies. final options = RequestOptions(baseUrl: exampleUrl); - final mockRequestInterceptorHandler = - MockRequestInterceptorHandler(expectResult); - await cookieManager.onRequest( - options, - mockRequestInterceptorHandler, - ); + await expectRequestCookies(cookieManager, options, expectResult); }); test('can be saved to the location', () async { @@ -427,6 +623,25 @@ void main() { }); } +class _CookieRecordingAdapter implements HttpClientAdapter { + final List cookieHeaders = []; + final List requestOptions = []; + + @override + Future fetch( + RequestOptions options, + Stream? requestStream, + Future? cancelFuture, + ) async { + requestOptions.add(options); + cookieHeaders.add(options.headers[HttpHeaders.cookieHeader] as String?); + return ResponseBody.fromString('', HttpStatus.ok); + } + + @override + void close({bool force = false}) {} +} + class _RedirectAdapter implements HttpClientAdapter { final HttpClientAdapter _adapter = IOHttpClientAdapter(); From 4796267cea2d472fb26ef6645b159877f252b963 Mon Sep 17 00:00:00 2001 From: Alex Li Date: Tue, 21 Jul 2026 00:31:46 +0800 Subject: [PATCH 2/2] docs(cookie_manager): clarify interceptor ordering Document the URI and Cookie header ordering requirements for reused request options. Co-Authored-By: Codex --- plugins/cookie_manager/lib/src/cookie_mgr.dart | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/plugins/cookie_manager/lib/src/cookie_mgr.dart b/plugins/cookie_manager/lib/src/cookie_mgr.dart index 6b29de3a7..b904d9c1a 100644 --- a/plugins/cookie_manager/lib/src/cookie_mgr.dart +++ b/plugins/cookie_manager/lib/src/cookie_mgr.dart @@ -29,6 +29,11 @@ class _CookieHeaderState { } /// Cookie manager for HTTP requests based on [CookieJar]. +/// +/// Register this after interceptors that may change [RequestOptions.uri] or +/// the Cookie request header. Cookies are selected from the URI and reused +/// request options are recognized only while the header exactly matches this +/// manager's previous output. class CookieManager extends Interceptor { CookieManager( this.cookieJar, { @@ -154,6 +159,12 @@ class CookieManager extends Interceptor { } /// Load cookies in cookie string for the request. + /// + /// State is scoped to the identity of [options]. When the same instance is + /// reused, its original Cookie header is restored only if the current header + /// exactly matches the previous result of this method. Any other header is + /// treated as a new source. Incrementally modifying a generated header after + /// this manager can therefore cause saved cookies to be merged again. Future loadCookies(RequestOptions options) async { final savedCookies = await cookieJar.loadForRequest(options.uri); final previousCookies =