Sean-Walker0 opened a new pull request, #7329:
URL: https://github.com/apache/shenyu/pull/7329

   <!-- Describe your PR here; e.g. Fixes #issueNo -->
   Found by code audit (no existing issue — happy to file one if maintainers 
prefer).
   
   `ConsulSyncDataService#watchConfigKeyValues` gates dispatch on 
`!consulIndexes.containsValue(newIndex)`. `consulIndexes` holds **one entry per 
watch root** (7 roots), but Consul's `X-Consul-Index` is a **cluster-wide raft 
index**, so two roots polled after the same write burst legitimately observe 
the *same* new index. The first root to poll stores it; every other root then 
hits the `containsValue` clause, is silently skipped, and still has its index 
advanced at the end — its changed keys are never dispatched to `updateHandler` 
and its removed keys never reach `deleteHandler`. Gateways stay stale until a 
full reload.
   
   Per-root advancement is already correctly checked immediately above 
(`Objects.equals(newIndex, currentIndex)` → reschedule), so `containsValue` 
adds nothing except the cross-root drop; the per-key `modifyIndex`/md5 
comparisons inside the dispatch loop already filter out unchanged data when an 
unrelated root's index advances.
   
   <!--
   Thank you for proposing a pull request. This template will guide you through 
the essential steps necessary for a pull request.
   -->
   Make sure that:
   
   - [x] You have read the [contribution 
guidelines](https://shenyu.apache.org/community/contributor-guide).
   - [x] You submit test cases (unit or integration tests) that back your 
changes.
   - [x] Your local test passed `./mvnw test -pl 
shenyu-sync-data-center/shenyu-sync-data-consul -am and ./mvnw checkstyle:check 
-pl shenyu-sync-data-center/shenyu-sync-data-consul` (module-scoped; full build 
left to CI).
   
   ### Modifications
   
   - Delete the `!this.consulIndexes.containsValue(newIndex)` clause, leaving 
the existing per-root index-advance check and the initial-poll 
(`INIT_CONFIG_VERSION_INDEX`) guard.
   
   ### Verifying this change
   
   - New `watchConfigKeyValuesShouldDispatchWhenAnotherRootSharesTheNewIndex` 
seeds `/shenyu/plugin` at index 20 and `/shenyu/selector` at 10, returns a 
changed selector KV with global index 20, and asserts the selector root 
dispatches. It fails on current master (dispatch list empty) and passes with 
this change.
   - Full `shenyu-sync-data-consul` module suite green; checkstyle green.
   
   ### Notes
   
   - Behavior change: a root whose own index advanced now dispatches even when 
another root already stored that global index; unchanged keys in the same 
response are still filtered per-key, so the only extra work is an occasional 
redundant rescan of that root's own keys.
   - Orthogonal to open PRs: no open PR touches `ConsulSyncDataService`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to