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 2021/07/25 20:15:26 UTC
[GitHub] [zookeeper] ztzg commented on a change in pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17
ztzg commented on a change in pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724#discussion_r676195353
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
##########
@@ -224,12 +228,24 @@ public OCSPHandler(X509Certificate revokedCert) {
public void handle(com.sun.net.httpserver.HttpExchange httpExchange) throws IOException {
byte[] responseBytes;
try {
+ String uri = httpExchange.getRequestURI().toString();
+ LOG.info("OCSP request: {} {} {}", httpExchange.getRequestMethod(), uri, httpExchange.getRequestHeaders().entrySet());
+ httpExchange.getRequestHeaders().entrySet().forEach((e) -> {
+ LOG.info("OCSP request header: {} {}", e.getKey(), e.getValue());
+ });
Review comment:
This logs each header twice: once at the end of the `OCSP request: ` line, in "compressed form," and once per line in the rest. We could perhaps drop the third format argument from the first line?
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
##########
@@ -224,12 +228,24 @@ public OCSPHandler(X509Certificate revokedCert) {
public void handle(com.sun.net.httpserver.HttpExchange httpExchange) throws IOException {
byte[] responseBytes;
try {
+ String uri = httpExchange.getRequestURI().toString();
+ LOG.info("OCSP request: {} {} {}", httpExchange.getRequestMethod(), uri, httpExchange.getRequestHeaders().entrySet());
+ httpExchange.getRequestHeaders().entrySet().forEach((e) -> {
+ LOG.info("OCSP request header: {} {}", e.getKey(), e.getValue());
+ });
InputStream request = httpExchange.getRequestBody();
byte[] requestBytes = new byte[10000];
- request.read(requestBytes);
+ int len = request.read(requestBytes);
+ LOG.info("OCSP request size {}: {}", len, new String(requestBytes, StandardCharsets.UTF_8));
Review comment:
This dumps 10000 bytes of binary data in the log. (Up to 10000 NUL bytes, if `len` is -1.) May I suggest dropping the second format argument, as it is unreadable even when the OCSP request is passed in the body (the data does not seem to be UTF-8 encoded anyway)?
##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
##########
@@ -224,12 +228,24 @@ public OCSPHandler(X509Certificate revokedCert) {
public void handle(com.sun.net.httpserver.HttpExchange httpExchange) throws IOException {
byte[] responseBytes;
try {
+ String uri = httpExchange.getRequestURI().toString();
+ LOG.info("OCSP request: {} {} {}", httpExchange.getRequestMethod(), uri, httpExchange.getRequestHeaders().entrySet());
+ httpExchange.getRequestHeaders().entrySet().forEach((e) -> {
+ LOG.info("OCSP request header: {} {}", e.getKey(), e.getValue());
+ });
InputStream request = httpExchange.getRequestBody();
byte[] requestBytes = new byte[10000];
- request.read(requestBytes);
+ int len = request.read(requestBytes);
+ LOG.info("OCSP request size {}: {}", len, new String(requestBytes, StandardCharsets.UTF_8));
+ if (len < 0) {
+ String removedUriEncodiing = URLDecoder.decode(uri.substring(1), "utf-8");
Review comment:
Nit/typo: `removedUriEncoding`.
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org