gharris1727 commented on code in PR #16754: URL: https://github.com/apache/kafka/pull/16754#discussion_r1879053818
########## connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/util/SSLUtilsTest.java: ########## @@ -33,6 +36,22 @@ public class SSLUtilsTest { + private Map<String, Object> sslConfig; + private String keystorePath; + private String truststorePath; + private Password keystorePassword; + private Password truststorePassword; + + @BeforeEach + public void bofore() throws Exception { Review Comment: nit typo: bofore ########## connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/DedicatedMirrorIntegrationTest.java: ########## @@ -280,7 +280,12 @@ public void testMultiNodeCluster() throws Exception { // Cluster aliases final String a = "A"; // Use a convoluted cluster name to ensure URL encoding/decoding works - final String b = "B- ._~:/?#[]@!$&'()*+;=\"<>%{}|\\^`618"; + // TODO: Jetty 12 throws a 400 amiguous error with the previous test string here + // It seems like at least some encoded characters are not allowed anymore based on + // https://github.com/jetty/jetty.project/issues/11890#issuecomment-2156442947 + // See https://github.com/jetty/jetty.project/issues/11890 + //final String b = "B- ._~:/?#[]@!$&'()*+;=\"<>%{}|\\^`618"; + final String b = "B"; Review Comment: Could you keep most of these suspicious characters, and just remove the ones which are ambiguous? From reading the linked issue, it might just be the `/` character, and the remainder are still valid. It looks like this test was sensitive enough to find the underlying change in the library, so I'd like to keep it sensitive in case something like this happens again. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org