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/10 13:47:03 UTC

[GitHub] [zookeeper] eolivelli opened a new pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

eolivelli opened a new pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724


   https://issues.apache.org/jira/browse/ZOOKEEPER-4333
   
   in JDK17 the OCSP request is sent in the URI and not inside the POST BODY


-- 
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



[GitHub] [zookeeper] ztzg closed pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
ztzg closed pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724


   


-- 
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



[GitHub] [zookeeper] eolivelli commented on pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724#issuecomment-887285878


   @ztzg thanks for your review.
   I have addressed your comments
   PTAL (and hopefully merge)
   please cherry pick to branch-3.7 it is better that at least 3.7 passes the tests on modern JDKs


-- 
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



[GitHub] [zookeeper] ztzg commented on pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
ztzg commented on pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724#issuecomment-885397433


   > kindly pinging @ztzg @maoling
   
   Hi @eolivelli,
   
   This is in my queue, and I am back in Munich.  I should be able to look into this on Saturday.
   
   Cheers, -D


-- 
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



[GitHub] [zookeeper] eolivelli commented on pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724#issuecomment-887462786


   @ztzg  CI passed


-- 
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



[GitHub] [zookeeper] ztzg commented on a change in pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
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



[GitHub] [zookeeper] eolivelli commented on pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724#issuecomment-881489269


   kindly pinging @ztzg @maoling 


-- 
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



[GitHub] [zookeeper] ztzg commented on pull request #1724: ZOOKEEPER-4333 QuorumSSLTest - testOCSP fails on JDK17

Posted by GitBox <gi...@apache.org>.
ztzg commented on pull request #1724:
URL: https://github.com/apache/zookeeper/pull/1724#issuecomment-888302645


   @eolivelli: This is now in `master` and `branch-3.7`.  Cheers!


-- 
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