Sean-Walker0 opened a new pull request, #7385: URL: https://github.com/apache/shenyu/pull/7385
<!-- Describe your PR here; e.g. Fixes #issueNo --> Companion to #7334 — its review flagged this exact exposure: `ClusterSelectMasterServiceZookeeperImpl#getMasterUrl` "has the identical exposure, one level deeper ... Both are worth a companion PR so the two `ClusterSelectMasterService` implementations behave identically". Found by code audit (no existing issue; happy to file one if maintainers prefer). `ClusterSelectMasterServiceZookeeperImpl#getMasterUrl` dereferences the deserialized master info unconditionally: - **znode absent** — `ClusterZookeeperClient#getDirectly` wraps Curator's `NoNode` in `ShenyuException`, which escapes to the websocket handler thread; - **znode empty or unparsable** — `JsonUtils#jsonToObject` catches the `IOException` and returns `null`, and `master.getContextPath()` throws `NullPointerException`. Both are the anticipated state of a freshly started Zookeeper-mode cluster: the `/shenyu_cluster_lock/master/info` znode is only written by the first successful `selectMaster(host, port, contextPath)`. `WebsocketCollector#onMessage` calls `getMasterUrl()` when answering every `RUNNING_MODE` request, so a node can fail inside the websocket thread before any master is elected. The JDBC implementation guards the same window (#7334: absent row → `StringUtils.EMPTY`). Separately, the two implementations disagreed on a leading slash: the JDBC sibling appends a `contextPath` that already starts with `/` as-is, while this one always prepended `/`, producing `schema://host:port//path` for the usual `server.servlet.context-path` form that `selectMaster` writes verbatim. <!-- 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-admin -am` (1624 tests green) and `./mvnw checkstyle:check -pl shenyu-admin` (module-scoped; full build left to CI). ### Modifications - `getMasterUrl()` returns `StringUtils.EMPTY` when the info znode is absent (pre-check via the existing `isExist`) or when its content does not deserialize — mirroring the JDBC sibling's absent-row guard. Real ZooKeeper connectivity errors still propagate from `isExist`/`getDirectly`. - A `contextPath` starting with `/` is appended as-is, matching the JDBC implementation; bare and empty `contextPath` behavior is unchanged. ### Verifying this change New `ClusterSelectMasterServiceZookeeperImplTest` (mocks the curator-facing `ClusterZookeeperClient`, deserializes through the real `JsonUtils`): - `testGetMasterUrlShouldNotThrowWhenMasterInfoNodeIsAbsent` — fails on current master with the exact `ShenyuException: NoNode`. - `testGetMasterUrlShouldNotThrowWhenMasterInfoContentIsEmpty` — fails on current master with the exact `NullPointerException` on `master.getContextPath()`. - `testGetMasterUrlShouldNotDoubleTheSlashWhenContextPathStartsWithOne` — fails on current master with `...//admin` instead of `.../admin`. - `testGetMasterUrlWithBareContextPath` / `testGetMasterUrlWithoutContextPath` — controls pinning the unchanged behavior (green before and after). ### Notes - Behavior change: absent/unreadable master info no longer throws inside the websocket thread; the `RUNNING_MODE` answer now carries an empty master URL until the first `selectMaster` writes the znode — the same contract #7334 established for the JDBC implementation. - Incidental finding (not touched here): this implementation's `getMaster()` returns `null` where the JDBC sibling returns an empty DTO; no caller dereferences it today, left unchanged to keep this PR minimal. - Orthogonality: no open PR modifies `ClusterSelectMasterServiceZookeeperImpl` (verified by keyword search plus per-PR file audit of the cluster/websocket-scoped PRs); #7334 touches only the JDBC implementation, this PR only the Zookeeper one. -- 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]
