fix: Correct conditional-request header and sanitize network error log in FDv1 polling requestor

packages/common_client/lib/src/data_sources/polling_data_source.dart

@@ -95,8 +95,11 @@ final class PollingDataSource implements DataSource {
 
     switch (event) {
       case null:
-        // No change.
-        return;
+        // No change (e.g. 304 Not Modified). Fall through to schedule
+        // the next poll -- returning here would leave the timer
+        // unset and silently stop polling after the first unchanged
+        // response.
+        break;
       case DataEvent():
         _eventController.sink.add(event);
       case StatusEvent():

packages/common_client/lib/src/data_sources/requestor.dart

@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:http/http.dart' as http;
 import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart';
 
@@ -57,25 +59,42 @@ final class Requestor {
   Future<DataSourceEvent?> requestAllFlags() async {
     try {
       _logger.debug(
-          'Making polling request, method: $_method, uri: $_uri, etag: $_lastEtag');
+          'Making polling request, method: $_method, hasEtag: ${_lastEtag != null}');
+      final additionalHeaders = switch (_lastEtag) {
+        final etag? => {'if-none-match': etag},
+        null => null,
+      };
       final res = await _client.request(_method, _uri,
-          additionalHeaders: _lastEtag != null ? {'etag': _lastEtag!} : null,
+          additionalHeaders: additionalHeaders,
           body: _method != RequestMethod.get ? _contextString : null);
       return await _handleResponse(res);
     } catch (err) {
-      return StatusEvent(ErrorKind.networkError, null, err.toString());
+      // Don't echo the raw exception. `http.ClientException`'s
+      // `toString()` formats as `'ClientException: <msg>, uri=<full-url>'`,
+      // and the URL embeds the base64url-encoded context in GET mode.
+      // Categorize into a fixed message and use that everywhere.
+      final sanitized = _describeError(err);
+      _logger.warn('Polling request failed: $sanitized');
+      return StatusEvent(ErrorKind.networkError, null, sanitized);
     }
   }
 
   Future<DataSourceEvent?> _handleResponse(http.Response res) async {
-    if (res.statusCode == 200 || res.statusCode == 304) {
+    if (res.statusCode == 304) {
+      // Server confirmed our cached payload is still current.
+      return null;
+    }
+    if (res.statusCode == 200) {
+      // Reject empty-string ETags: an unquoted empty token is invalid
+      // per RFC 7232 §2.1, and sending `if-none-match: ` on the next
+      // request could be interpreted by some servers as "match anything"
+      // and pin the SDK to a permanent 304. Also preserve the previously
+      // stored value when the response simply omits the header rather
+      // than clearing it.
       final etag = res.headers['etag'];
-      if (etag != null && etag == _lastEtag) {
-        // The response has not changed, so we don't need to do the work of
-        // updating the store, calculating changes, or persisting the payload.
-        return null;
+      if (etag != null && etag.isNotEmpty) {
+        _lastEtag = etag;
       }
-      _lastEtag = etag;
 
       var environmentId = getEnvironmentId(res.headers);
 
@@ -88,15 +107,25 @@ final class Requestor {
       }
 
       return DataEvent('put', res.body, environmentId: environmentId);
-    } else {
-      if (isHttpGloballyRecoverable(res.statusCode)) {
-        return StatusEvent(ErrorKind.errorResponse, res.statusCode,
-            'Received unexpected status code: ${res.statusCode}');
-      } else {
-        return StatusEvent(ErrorKind.errorResponse, res.statusCode,
-            'Received unexpected status code: ${res.statusCode}',
-            shutdown: true);
-      }
     }
+    if (isHttpGloballyRecoverable(res.statusCode)) {
+      return StatusEvent(ErrorKind.errorResponse, res.statusCode,
+          'Received unexpected status code: ${res.statusCode}');
+    }
+    return StatusEvent(ErrorKind.errorResponse, res.statusCode,
+        'Received unexpected status code: ${res.statusCode}',
+        shutdown: true);
+  }
+
+  String _describeError(Object err) {
+    if (err is TimeoutException) return 'Polling request timed out';
+    if (err is http.ClientException) {
+      return 'Network error during polling request';
+    }
+    final type = err.runtimeType.toString();
+    if (type.contains('Tls') || type.contains('Handshake')) {
+      return 'TLS error during polling request';
+    }
+    return 'Polling request failed';
   }
 }

packages/common_client/test/data_sources/polling_data_source_test.dart

@@ -12,21 +12,25 @@ import 'package:launchdarkly_common_client/src/flag_manager/flag_manager.dart';
 import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'
     as ld_common;
 import 'package:http/http.dart' as http;
+import 'package:mocktail/mocktail.dart';
 
 import 'package:test/test.dart';
 
+class MockLogAdapter extends Mock implements LDLogAdapter {}
+
 (PollingDataSource, FlagManager, DataSourceStatusManager) makeDataSourceForTest(
     MockClient innerClient,
     {LDContext? inContext,
     HttpProperties? inProperties,
     bool useReport = false,
     bool withReasons = false,
-    Duration? testingInterval}) {
+    Duration? testingInterval,
+    LDLogger? inLogger}) {
   final context = inContext ?? LDContextBuilder().kind('user', 'test').build();
   // We are not testing the data source status manager here, so we just want a
   // fixed time to make events easy to get.
   final statusManager = DataSourceStatusManager(stamper: () => DateTime(1));
-  final logger = LDLogger();
+  final logger = inLogger ?? LDLogger();
   final httpProperties = inProperties ?? HttpProperties();
   const sdkKey = 'dummy-key';
   final flagManager =
@@ -511,4 +515,207 @@ void main() {
 
     polling.stop();
   });
+
+  group('conditional requests', () {
+    setUpAll(() {
+      registerFallbackValue(LDLogRecord(
+          level: LDLogLevel.debug,
+          message: '',
+          time: DateTime.now(),
+          logTag: ''));
+    });
+
+    test(
+        'sends if-none-match on the next request after receiving a 200 with etag',
+        () async {
+      var requestNumber = 0;
+      Map<String, String>? secondRequestHeaders;
+      final innerClient = MockClient((request) async {
+        requestNumber++;
+        if (requestNumber == 1) {
+          return http.Response('{}', 200, headers: {'etag': 'abc-123'});
+        }
+        secondRequestHeaders = request.headers;
+        return http.Response('{}', 200);
+      });
+
+      final (polling, _, _) = makeDataSourceForTest(innerClient,
+          testingInterval: const Duration(milliseconds: 5));
+      polling.start();
+
+      // Drain to the second request before asserting.
+      while (requestNumber < 2) {
+        await Future<void>.delayed(const Duration(milliseconds: 5));
+      }
+      polling.stop();
+
+      expect(secondRequestHeaders, containsPair('if-none-match', 'abc-123'));
+      // The wrong header name (`etag`) must not be sent.
+      expect(secondRequestHeaders!.containsKey('etag'), isFalse);
+    });
+
+    test('empty-string etag on a 200 response is not stored', () async {
+      var requestNumber = 0;
+      Map<String, String>? secondRequestHeaders;
+      final innerClient = MockClient((request) async {
+        requestNumber++;
+        if (requestNumber == 1) {
+          return http.Response('{}', 200, headers: {'etag': ''});
+        }
+        secondRequestHeaders = request.headers;
+        return http.Response('{}', 200);
+      });
+
+      final (polling, _, _) = makeDataSourceForTest(innerClient,
+          testingInterval: const Duration(milliseconds: 5));
+      polling.start();
+
+      while (requestNumber < 2) {
+        await Future<void>.delayed(const Duration(milliseconds: 5));
+      }
+      polling.stop();
+
+      // An unquoted empty token is invalid per RFC 7232 and sending
+      // `if-none-match: ` could pin the SDK to permanent 304s on lenient
+      // servers. The empty value must not round-trip.
+      expect(secondRequestHeaders!.containsKey('if-none-match'), isFalse);
+    });
+
+    test('200 without an etag header preserves the previously stored value',
+        () async {
+      var requestNumber = 0;
+      Map<String, String>? thirdRequestHeaders;
+      final innerClient = MockClient((request) async {
+        requestNumber++;
+        if (requestNumber == 1) {
+          return http.Response('{}', 200, headers: {'etag': 'first'});
+        }
+        if (requestNumber == 2) {
+          // 200 without an etag header. Old code would clear `_lastEtag`
+          // to null here; the fix preserves "first".
+          return http.Response('{}', 200);
+        }
+        thirdRequestHeaders = request.headers;
+        return http.Response('{}', 200);
+      });
+
+      final (polling, _, _) = makeDataSourceForTest(innerClient,
+          testingInterval: const Duration(milliseconds: 5));
+      polling.start();
+
+      while (requestNumber < 3) {
+        await Future<void>.delayed(const Duration(milliseconds: 5));
+      }
+      polling.stop();
+
+      expect(thirdRequestHeaders, containsPair('if-none-match', 'first'));
+    });
+
+    test('304 response is treated as no-change, not as malformed data',
+        () async {
+      // The pre-fix behavior fell through 304 to DataEvent('put', '', ...),
+      // which downstream parsed as empty JSON and surfaced as
+      // `ErrorKind.invalidData`. The fix returns null on 304, so the
+      // status manager should never see an `invalidData` error.
+      var requestCount = 0;
+      final innerClient = MockClient((request) async {
+        requestCount++;
+        return http.Response('', 304);
+      });
+
+      final (polling, flagManager, statusManager) = makeDataSourceForTest(
+          innerClient,
+          testingInterval: const Duration(milliseconds: 5));
+      polling.start();
+
+      while (requestCount < 1) {
+        await Future<void>.delayed(const Duration(milliseconds: 5));
+      }
+      // Give the status pipeline a chance to surface anything.
+      await Future<void>.delayed(const Duration(milliseconds: 20));
+      polling.stop();
+
+      final lastError = statusManager.status.lastError;
+      expect(lastError?.kind, isNot(equals(ErrorKind.invalidData)));
+      // FlagManager should remain in its initial empty state -- no
+      // attempt was made to parse the empty body.
+      expect(flagManager.environmentId, isNull);
+    });
+
+    test('a 304 response does not stop polling', () async {
+      // Regression: a previous version of _doPoll early-returned on a
+      // null event without scheduling the next poll, so the first 304
+      // permanently halted the loop. This pins multiple 304s in a row
+      // resulting in multiple requests.
+      var requestCount = 0;
+      final innerClient = MockClient((request) async {
+        requestCount++;
+        return http.Response('', 304);
+      });
+
+      final (polling, _, _) = makeDataSourceForTest(innerClient,
+          testingInterval: const Duration(milliseconds: 5));
+      polling.start();
+
+      while (requestCount < 3) {
+        await Future<void>.delayed(const Duration(milliseconds: 5));
+      }
+      polling.stop();
+
+      expect(requestCount, greaterThanOrEqualTo(3));
+    });
+  });
+
+  group('network error log content', () {
+    setUpAll(() {
+      registerFallbackValue(LDLogRecord(
+          level: LDLogLevel.debug,
+          message: '',
+          time: DateTime.now(),
+          logTag: ''));
+    });
+
+    test(
+        'warn-level log on a network error does not contain the encoded context',
+        () async {
+      // http.ClientException's toString embeds the request URL, which
+      // in turn embeds the base64url-encoded context. The requestor
+      // must categorize the exception into a fixed string and log
+      // only that.
+      final adapter = MockLogAdapter();
+      when(() => adapter.log(any())).thenReturn(null);
+      final logger = LDLogger(adapter: adapter, level: LDLogLevel.debug);
+
+      final secretContext =
+          LDContextBuilder().kind('user', 'secret-key-shibboleth').build();
+      final innerClient = MockClient((request) async {
+        throw http.ClientException('Connection refused', request.url);
+      });
+
+      final (polling, _, statusManager) = makeDataSourceForTest(innerClient,
+          inContext: secretContext,
+          inLogger: logger,
+          testingInterval: const Duration(milliseconds: 1));
+      polling.start();
+      await Future<void>.delayed(const Duration(milliseconds: 30));
+      polling.stop();
+
+      final encodedContext = base64UrlEncode(utf8.encode(jsonEncode(
+          ld_common.LDContextSerialization.toJson(secretContext,
+              isEvent: false))));
+      final records = verify(() => adapter.log(captureAny())).captured;
+      for (final record in records) {
+        expect(
+            (record as LDLogRecord).message, isNot(contains(encodedContext)));
+        expect(record.message, isNot(contains('secret-key-shibboleth')));
+      }
+
+      // The user-visible status surface must also not echo the URL.
+      final lastError = statusManager.status.lastError;
+      if (lastError != null) {
+        expect(lastError.message, isNot(contains(encodedContext)));
+        expect(lastError.message, isNot(contains('secret-key-shibboleth')));
+      }
+    });
+  });
 }