> On March 8, 2017, 1:16 p.m., Jinmei Liao wrote: > > Any test changes? We probably can create a integrated/dunit test that would > > start a server with those ssl properties (including passwords) turned on, > > and have debug level truned on, and security truned on as well, and have > > gfsh connect to it using username and password, and see if any of the > > password show up in the logs. > > Kevin Duling wrote: > `ArgumentRedactorJUnitTest` already has 85% method coverage and 95% line > coverage of `ArgumentRedactor`. `SocketCreator.printConfig()` is a void > method which only writes out to the log file. I could add a specific test > for the expected ssl property, but it's already covered by the comparison > `compareKey.toLowerCase().contains("password");` > > Jinmei Liao wrote: > ArugmentRedactor is defintely doing the right thing since we have test > coverage on that. It's the SocketCreator that is the problem here.
I disagree. ArgumentRedactor just has to populate the StringBuilder with the correct string. The only additional piece is calling the logger to write the file, which is Log4J, not SocketCreator. I'm not clear on what additional testing would prove. `RestSecurityWithSSLTest` is probably a good test to work from as it is an entry point for this code. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57431/#review168333 ----------------------------------------------------------- On March 8, 2017, 12:58 p.m., Kevin Duling wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57431/ > ----------------------------------------------------------- > > (Updated March 8, 2017, 12:58 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, and Kirk Lund. > > > Repository: geode > > > Description > ------- > > GEODE-2633: When turning on fine logging, GEODE logs the keystore password in > clear text > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java > 742e7f3e93e595844675d0789b995e4ceb4431ac > > geode-core/src/main/java/org/apache/geode/internal/util/ArgumentRedactor.java > 419f3f976159e601c95a2042bafd96cc9fe9465f > > > Diff: https://reviews.apache.org/r/57431/diff/1/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Kevin Duling > >