[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-26 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16027029#comment-16027029
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/240
  
@afine looks like one test 
(org.apache.zookeeper.ZooKeeperTest.testCreateNodeWithoutData) is broken 
because of this patch. please check upstream build, should be pretty trivial to 
fix. Wondering why the pre-commit build does not catch this test.


> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4, 3.6.0
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-26 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16026985#comment-16026985
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/240


> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4, 3.6.0
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-24 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16022364#comment-16022364
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

Github user afine commented on the issue:

https://github.com/apache/zookeeper/pull/240
  
@hanm I don't think it is ever safe to make assumptions about the state of 
the process after a bug/"programming problem" has been encountered. So I think 
its better to exit the process.

I fixed the compiler warning.


> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-24 Thread Hadoop QA (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16022366#comment-16022366
 ] 

Hadoop QA commented on ZOOKEEPER-2757:
--

-1 overall.  GitHub Pull Request  Build
  

+1 @author.  The patch does not contain any @author tags.

+0 tests included.  The patch appears to be a documentation patch that 
doesn't require tests.

+1 javadoc.  The javadoc tool did not generate any warning messages.

+1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

+1 findbugs.  The patch does not introduce any new Findbugs (version 3.0.1) 
warnings.

+1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

-1 core tests.  The patch failed core unit tests.

+1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/741//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/741//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/741//console

This message is automatically generated.

> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-23 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16022319#comment-16022319
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/240
  
Agree with what you said, Abe, in a generic context : ) - for CLI use cases 
though I am wondering why we should crash even in cases as you mentioned "So if 
it does pop up for reasons other than a bad path" - in such case should we 
always print the error message and not crashing the CLI? Would that be more 
helpful for user experience comparing to crash and leave no ideas to users what 
went wrong? Basically my point is that I don't any use case where the CLI's 
state is so bad that it can't recover from an unchecked exception so it has to 
be killed. 

Other than this patch looks good to me. One nit - a new compiler waring is 
introduced.


> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-18 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16016929#comment-16016929
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

Github user afine commented on the issue:

https://github.com/apache/zookeeper/pull/240
  
@hanm I'm not sure that we should always assume that an 
IllegalArgumentException will only be thrown when a path is invalid. Since 
IllegalArgumentException does not need to be caught, it will be difficult for 
us to tell if we are masking IllegalArgumentExceptions thrown for other 
reasons. So we should minimize the code surrounded by `try { } 
catch(IllegalArgumentException e) {}` We cannot catch them within the methods 
of ZooKeeper.java so I think catching them at the "Command" level is the best 
we can do.

IllegalArgumentException is a RuntimeException, which should represent a 
"programming problem" 
(http://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html). 
>Runtime exceptions represent problems that are the result of a programming 
problem, and as such, the API client code cannot reasonably be expected to 
recover from them or to handle them in any way. 

So if it does pop up for reasons other than a bad path, we should certainly 
bubble it up and kill the CLI.

We abuse IllegalArgumentException all over ZK, I believe we should change 
this exception type in 3.6.

 


> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-05-18 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16016650#comment-16016650
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

Github user hanm commented on the issue:

https://github.com/apache/zookeeper/pull/240
  
How about catch the IllegalArgumentException at ZooKeeperMain.processCmd 
and directly print the exception message? This way we don't need to make change 
to individual command.


> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-04-24 Thread Hadoop QA (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15982305#comment-15982305
 ] 

Hadoop QA commented on ZOOKEEPER-2757:
--

-1 overall.  GitHub Pull Request  Build
  

+1 @author.  The patch does not contain any @author tags.

+0 tests included.  The patch appears to be a documentation patch that 
doesn't require tests.

+1 javadoc.  The javadoc tool did not generate any warning messages.

+1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

+1 findbugs.  The patch does not introduce any new Findbugs (version 3.0.1) 
warnings.

+1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

-1 core tests.  The patch failed core unit tests.

+1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/602//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/602//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/602//console

This message is automatically generated.

> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (ZOOKEEPER-2757) Incorrect path crashes zkCli

2017-04-24 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15981978#comment-15981978
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2757:
---

GitHub user afine opened a pull request:

https://github.com/apache/zookeeper/pull/240

ZOOKEEPER-2757: Incorrect path crashes zkCli

This issue is caused by us relying on `IllegalArgumentException` in 
`PathUtils#validatePath`. `IllegalArgumentException` is an unchecked exception 
and we never catch it within each individual *Command.java, so it bubbles up 
and killed the CLI. 

Given that throwing `IllegalArgumentException` is part of ZooKeeper's API, 
I believe that unfortunately we can not change this behavior at this time. This 
patch catches `IllegalArgumentException` and wraps it, so the CLI prints an 
error but does not quit. I believe I handled all of the relevant commands, 
please check to make sure I am not missing one.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/afine/zookeeper ZOOKEEPER-2757

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/240.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #240


commit 46dce3f1c5c219b72e4e046c0bb42c1201d44238
Author: Abraham Fine 
Date:   2017-04-24T21:47:29Z

ZOOKEEPER-2757: Incorrect path crashes zkCli




> Incorrect path crashes zkCli
> 
>
> Key: ZOOKEEPER-2757
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2757
> Project: ZooKeeper
>  Issue Type: Bug
>Affects Versions: 3.5.3
>Reporter: Flavio Junqueira
>Assignee: Abraham Fine
>Priority: Minor
> Fix For: 3.5.4
>
>
> If I try {{delete test}} without the leading /, then the CLI crashes with 
> this exception:
> {noformat}
> Exception in thread "main" java.lang.IllegalArgumentException: Path must 
> start with / character
>   at org.apache.zookeeper.common.PathUtils.validatePath(PathUtils.java:51)
>   at org.apache.zookeeper.ZooKeeper.delete(ZooKeeper.java:1659)
>   at org.apache.zookeeper.cli.DeleteCommand.exec(DeleteCommand.java:83)
>   at 
> org.apache.zookeeper.ZooKeeperMain.processZKCmd(ZooKeeperMain.java:655)
>   at org.apache.zookeeper.ZooKeeperMain.processCmd(ZooKeeperMain.java:586)
>   at 
> org.apache.zookeeper.ZooKeeperMain.executeLine(ZooKeeperMain.java:370)
>   at org.apache.zookeeper.ZooKeeperMain.run(ZooKeeperMain.java:330)
>   at org.apache.zookeeper.ZooKeeperMain.main(ZooKeeperMain.java:292)
> {noformat}
> It should really fail the operation rather than crash the CLI.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)