You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/11/09 00:46:20 UTC
[GitHub] [zookeeper] mino181295 opened a new pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
mino181295 opened a new pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530
Sensitive data about args[0], zkHostPort, and cmdNumber are directly printed and may leak.
For security, log should be used to record these data, as well as log in other classes such as org.apache.zookeeper.server.ZooKeeperServer:
LOG = LoggerFactory.getLogger(GenerateLoad.class);
```
LOG.error("Could not connect to " + args[0]);
......
LOG.info("Connecting exclusively to " + zkHostPort.toString());
......
LOG.error("Not a valid number: " + e.getMessage());
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] mino181295 commented on a change in pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
mino181295 commented on a change in pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#discussion_r520444719
##########
File path: zookeeper-it/src/main/java/org/apache/zookeeper/test/system/GenerateLoad.java
##########
@@ -713,7 +711,7 @@ private static String getMode(String hostPort) throws NumberFormatException, Unk
}
private static void doUsage() {
- System.err.println("USAGE: " + GenerateLoad.class.getName()
+ LOG.info("USAGE: " + GenerateLoad.class.getName()
Review comment:
I agree with @ztzg do you think it should be `System.err.println` also line 673-676?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] breed commented on a change in pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
breed commented on a change in pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#discussion_r527204921
##########
File path: zookeeper-it/src/main/java/org/apache/zookeeper/test/system/GenerateLoad.java
##########
@@ -713,7 +711,7 @@ private static String getMode(String hostPort) throws NumberFormatException, Unk
}
private static void doUsage() {
- System.err.println("USAGE: " + GenerateLoad.class.getName()
+ LOG.info("USAGE: " + GenerateLoad.class.getName()
Review comment:
oh and i see that it has been addressed :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ztzg commented on a change in pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
ztzg commented on a change in pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#discussion_r519725701
##########
File path: zookeeper-it/src/main/java/org/apache/zookeeper/test/system/GenerateLoad.java
##########
@@ -713,7 +711,7 @@ private static String getMode(String hostPort) throws NumberFormatException, Unk
}
private static void doUsage() {
- System.err.println("USAGE: " + GenerateLoad.class.getName()
+ LOG.info("USAGE: " + GenerateLoad.class.getName()
Review comment:
I would say that the usage synopsis should still go to `System.err`. What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] breed commented on a change in pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
breed commented on a change in pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#discussion_r527203180
##########
File path: zookeeper-it/src/main/java/org/apache/zookeeper/test/system/GenerateLoad.java
##########
@@ -713,7 +711,7 @@ private static String getMode(String hostPort) throws NumberFormatException, Unk
}
private static void doUsage() {
- System.err.println("USAGE: " + GenerateLoad.class.getName()
+ LOG.info("USAGE: " + GenerateLoad.class.getName()
Review comment:
ah dang, i hit approve too quickly. i agree with this requested change as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ztzg commented on a change in pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
ztzg commented on a change in pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#discussion_r521194738
##########
File path: zookeeper-it/src/main/java/org/apache/zookeeper/test/system/GenerateLoad.java
##########
@@ -713,7 +711,7 @@ private static String getMode(String hostPort) throws NumberFormatException, Unk
}
private static void doUsage() {
- System.err.println("USAGE: " + GenerateLoad.class.getName()
+ LOG.info("USAGE: " + GenerateLoad.class.getName()
Review comment:
I'd say so. (Sorry for missing it on the first round.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#discussion_r519727189
##########
File path: zookeeper-it/src/main/java/org/apache/zookeeper/test/system/GenerateLoad.java
##########
@@ -713,7 +711,7 @@ private static String getMode(String hostPort) throws NumberFormatException, Unk
}
private static void doUsage() {
- System.err.println("USAGE: " + GenerateLoad.class.getName()
+ LOG.info("USAGE: " + GenerateLoad.class.getName()
Review comment:
I do not have a strong opinion here
@mino181295 what's your opinion ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ztzg commented on pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
ztzg commented on pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530#issuecomment-732110174
Thank you, @mino181295. This is now in `master`. (I see that the ticket references 3.4; would you also like this to go to older branches?)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ztzg closed pull request #1530: ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive…
Posted by GitBox <gi...@apache.org>.
ztzg closed pull request #1530:
URL: https://github.com/apache/zookeeper/pull/1530
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org