[jira] [Updated] (ZOOKEEPER-4565) Config watch path get truncated abnormally and fail chroot zookeeper client

["Kezhu Wang (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/ZOOKEEPER-4565?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kezhu Wang updated ZOOKEEPER-4565:
----------------------------------
    Description: 
{{ClientCnxn}} strips off {{chrootPath}} for notifications. This strip could break "/zookeeper/config" to illegal zookeeper path(aka. not start with "/") and fail zookeeper client due to index out of bound exception in {{PathParentIterator.next}}.

{code:java}
                // convert from a server path to a client path
                if (chrootPath != null) {
                    String serverPath = event.getPath();
                    if (serverPath.compareTo(chrootPath) == 0) {
                        event.setPath("/");
                    } else if (serverPath.length() > chrootPath.length()) {
                        event.setPath(serverPath.substring(chrootPath.length()));
                     } else {
                         LOG.warn("Got server path {} which is too short for chroot path {}.",
                             event.getPath(), chrootPath);
                     }
                }
{code}

I think we could make this strip similar to c client version {{sub_string}} and warns only for not "/zookeeper" nodes.

I found this issue in reproducing CURATOR-593 and CURATOR-611. And the consequences are different in 3.6.3 and before.

* Before 3.6.3(exclusive), watcher of {{getConfig}} will receive event with illegal path.
* After 3.6.3(inclusive), the client will disconnect due to exception from {{ZKWatchManager.addPersistentWatches}} which calls {{PathParentIterator.forAll}}. See codes from [3.6.3|https://github.com/apache/zookeeper/blob/release-3.6.3/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java#L627] and [3.7+|https://github.com/apache/zookeeper/blob/release-3.7.0/zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java#L450].

  was:
{{ClientCnxn}} strips off {{chrootPath}} for notifications. This strip could break "/zookeeper/config" to illegal zookeeper path(aka. not start with "/") and fail zookeeper client due to index out of bound exception in {{PathParentIterator.next}}.

{code:java}
                // convert from a server path to a client path
                if (chrootPath != null) {
                    String serverPath = event.getPath();
                    if (serverPath.compareTo(chrootPath) == 0) {
                        event.setPath("/");
                    } else if (serverPath.length() > chrootPath.length()) {
                        event.setPath(serverPath.substring(chrootPath.length()));
                     } else {
                         LOG.warn("Got server path {} which is too short for chroot path {}.",
                             event.getPath(), chrootPath);
                     }
                }
{code}

I think we could make this strip similar to c client version {{sub_string}} and warns only for not "/zookeeper" nodes.


> Config watch path get truncated abnormally and fail chroot zookeeper client
> ---------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-4565
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-4565
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: java client
>    Affects Versions: 3.6.3, 3.8.0, 3.7.1, 3.9.0
>            Reporter: Kezhu Wang
>            Assignee: Kezhu Wang
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> {{ClientCnxn}} strips off {{chrootPath}} for notifications. This strip could break "/zookeeper/config" to illegal zookeeper path(aka. not start with "/") and fail zookeeper client due to index out of bound exception in {{PathParentIterator.next}}.
> {code:java}
>                 // convert from a server path to a client path
>                 if (chrootPath != null) {
>                     String serverPath = event.getPath();
>                     if (serverPath.compareTo(chrootPath) == 0) {
>                         event.setPath("/");
>                     } else if (serverPath.length() > chrootPath.length()) {
>                         event.setPath(serverPath.substring(chrootPath.length()));
>                      } else {
>                          LOG.warn("Got server path {} which is too short for chroot path {}.",
>                              event.getPath(), chrootPath);
>                      }
>                 }
> {code}
> I think we could make this strip similar to c client version {{sub_string}} and warns only for not "/zookeeper" nodes.
> I found this issue in reproducing CURATOR-593 and CURATOR-611. And the consequences are different in 3.6.3 and before.
> * Before 3.6.3(exclusive), watcher of {{getConfig}} will receive event with illegal path.
> * After 3.6.3(inclusive), the client will disconnect due to exception from {{ZKWatchManager.addPersistentWatches}} which calls {{PathParentIterator.forAll}}. See codes from [3.6.3|https://github.com/apache/zookeeper/blob/release-3.6.3/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java#L627] and [3.7+|https://github.com/apache/zookeeper/blob/release-3.7.0/zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java#L450].



--
This message was sent by Atlassian Jira
(v8.20.10#820010)