You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2019/06/24 23:00:21 UTC

[lucene-solr] branch master updated: Revert "SOLR-12988: SSLTestConfig has been changed to throw AssumptionViolatedException when tests/seeds request SSL but the JVM appears to be an OpenJDK version known to have SSL bugs"

This is an automated email from the ASF dual-hosted git repository.

hossman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/master by this push:
     new 689fa58  Revert "SOLR-12988: SSLTestConfig has been changed to throw AssumptionViolatedException when tests/seeds request SSL but the JVM appears to be an OpenJDK version known to have SSL bugs"
689fa58 is described below

commit 689fa583a04d3aad56dba55abdff7ef968a8feff
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Mon Jun 24 15:42:23 2019 -0700

    Revert "SOLR-12988: SSLTestConfig has been changed to throw AssumptionViolatedException when tests/seeds request SSL but the JVM appears to be an OpenJDK version known to have SSL bugs"
    
    This reverts commit 6d6f14d39123512b8734d63c584bceb9d7bd832f.
    
    Reason for revert: after doing more testing I realized there are tests I overlooked which can (with randomized SSL usage) trigger NullPointerException
    (or other related failures) in After/AfterClass due assumptions about cleanup that isn't actaully neccessary due to the AssumptionFailure
    that may occur during Before/BeforeClass
---
 solr/CHANGES.txt                                   |  3 -
 .../solr/cloud/TestMiniSolrCloudClusterSSL.java    |  2 +
 .../apache/solr/cloud/TestSSLRandomization.java    |  2 +
 .../impl/HttpSolrClientSSLAuthConPoolTest.java     |  3 +
 .../java/org/apache/solr/util/SSLTestConfig.java   | 60 +++------------
 .../org/apache/solr/util/TestSSLTestConfig.java    | 87 ----------------------
 6 files changed, 16 insertions(+), 141 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0802311..992c218 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -202,9 +202,6 @@ Other Changes
 * SOLR-13511: Add SearchHandler.newResponseBuilder method to facilitate custom plugins' maintenance
   of per-request state in a custom ResponseBuilder. (Ramsey Haddad, Christine Poerschke)
 
-* SOLR-12988: SSLTestConfig has been changed to throw AssumptionViolatedException when tests/seeds
-  request SSL but the JVM appears to be an OpenJDK version known to have SSL bugs (hossman, Cao Manh Dat)
-
 ==================  8.1.2 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
index b659a1f..b9e8a04 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
@@ -85,6 +85,8 @@ public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 {
   
   @Before
   public void before() {
+    assumeFalse("@AwaitsFix: SOLR-12988 - ssl issues on Java 11/12", Constants.JRE_IS_MINIMUM_JAVA11);
+
     // undo the randomization of our super class
     log.info("NOTE: This Test ignores the randomized SSL & clientAuth settings selected by base class");
     HttpClientUtil.resetHttpClientBuilder(); // also resets SchemaRegistryProvider
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java b/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java
index e846f73..1241189 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java
@@ -19,6 +19,7 @@ package org.apache.solr.cloud;
 import java.lang.invoke.MethodHandles;
 import java.util.Arrays;
 
+import org.apache.lucene.util.Constants;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.util.SSLTestConfig;
 import org.apache.solr.util.RandomizeSSL;
@@ -43,6 +44,7 @@ public class TestSSLRandomization extends SolrCloudTestCase {
 
   @BeforeClass
   public static void createMiniSolrCloudCluster() throws Exception {
+    assumeFalse("@AwaitsFix: SOLR-12988 - ssl issues on Java 11/12", Constants.JRE_IS_MINIMUM_JAVA11);
     configureCluster(TestMiniSolrCloudClusterSSL.NUM_SERVERS).configure();
   }
   
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java
index cab94ac..3b59049 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientSSLAuthConPoolTest.java
@@ -20,6 +20,7 @@ package org.apache.solr.client.solrj.impl;
 import java.net.URL;
 import java.util.Arrays;
 
+import org.apache.lucene.util.Constants;
 import org.apache.solr.util.RandomizeSSL;
 import org.junit.BeforeClass;
 
@@ -29,6 +30,8 @@ public class HttpSolrClientSSLAuthConPoolTest extends HttpSolrClientConPoolTest
 
     @BeforeClass
     public static void checkUrls() throws Exception {
+      assumeFalse("@AwaitsFix: SOLR-12988 - ssl issues on Java 11/12", Constants.JRE_IS_MINIMUM_JAVA11);
+      
       URL[] urls = new URL[] {
           jetty.getBaseUrl(), yetty.getBaseUrl() 
       };
diff --git a/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java b/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java
index 88b6a1c..502df40 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java
@@ -25,8 +25,7 @@ import java.security.SecureRandom;
 import java.security.SecureRandomSpi;
 import java.security.UnrecoverableKeyException;
 import java.util.Random;
-import java.util.regex.Pattern;
-  
+
 import org.apache.http.config.Registry;
 import org.apache.http.config.RegistryBuilder;
 import org.apache.http.conn.socket.ConnectionSocketFactory;
@@ -44,8 +43,6 @@ import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.security.CertificateUtils;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 
-import com.carrotsearch.randomizedtesting.RandomizedTest;
-
 /**
  * An SSLConfig that provides {@link SSLConfig} and {@link SchemaRegistryProvider} for both clients and servers
  * that supports reading key/trust store information directly from resource files provided with the
@@ -59,8 +56,8 @@ public class SSLTestConfig {
   private final boolean checkPeerName;
   private final Resource keyStore;
   private final Resource trustStore;
-  private final boolean useSsl;
-  private final boolean clientAuth;
+  private boolean useSsl;
+  private boolean clientAuth;
   
   /** Creates an SSLTestConfig that does not use SSL or client authentication */
   public SSLTestConfig() {
@@ -102,14 +99,15 @@ public class SSLTestConfig {
    * @see HttpClientUtil#SYS_PROP_CHECK_PEER_NAME
    */
   public SSLTestConfig(boolean useSSL, boolean clientAuth, boolean checkPeerName) {
-    this.useSsl = useSSL;
+    // @AwaitsFix: SOLR-12988 - ssl issues on Java 11/12
+    if (Constants.JRE_IS_MINIMUM_JAVA11) {
+      this.useSsl = false;
+    } else {
+      this.useSsl = useSSL;
+    }
     this.clientAuth = clientAuth;
     this.checkPeerName = checkPeerName;
 
-    if (useSsl) {
-      assumeSslIsSafeToTest();
-    }
-    
     final String resourceName = checkPeerName
       ? TEST_KEYSTORE_LOCALHOST_RESOURCE : TEST_KEYSTORE_BOGUSHOST_RESOURCE;
     trustStore = keyStore = Resource.newClassPathResource(resourceName);
@@ -341,44 +339,4 @@ public class SSLTestConfig {
     synchronized public void setSeed(long seed) { /* NOOP */ }
     
   }
-
-  /**
-   * Helper method for sanity checking if it's safe to use SSL on this JVM
-   *
-   * @see <a href="https://issues.apache.org/jira/browse/SOLR-12988">SOLR-12988</a>
-   * @throws org.junit.internal.AssumptionViolatedException if this JVM is known to have SSL problems
-   */
-  public static void assumeSslIsSafeToTest() {
-    if (Constants.JVM_NAME.startsWith("OpenJDK") ||
-        Constants.JVM_NAME.startsWith("Java HotSpot(TM)")) {
-      RandomizedTest.assumeFalse("Test (or randomization for this seed) wants to use SSL, " +
-                                 "but SSL is known to fail on your JVM: " +
-                                 Constants.JVM_NAME + " / " + Constants.JVM_VERSION, 
-                                 isOpenJdkJvmVersionKnownToHaveProblems(Constants.JVM_VERSION));
-    }
-  }
-  
-  /** 
-   * package visibility for tests
-   * @see Constants#JVM_VERSION
-   * @lucene.internal
-   */
-  static boolean isOpenJdkJvmVersionKnownToHaveProblems(final String jvmVersion) {
-    // TODO: would be nice to replace with Runtime.Version once we don't have to
-    // worry about java8 support when backporting to branch_8x
-    return KNOWN_BAD_OPENJDK_JVMS.matcher(jvmVersion).matches();
-
-  }
-  private static final Pattern KNOWN_BAD_OPENJDK_JVMS
-    = Pattern.compile(// 11 to 11.0.2 were all definitely problematic
-                      // - https://bugs.openjdk.java.net/browse/JDK-8212885
-                      // - https://bugs.openjdk.java.net/browse/JDK-8213202
-                      "(^11(\\.0(\\.0|\\.1|\\.2)?)?($|(\\_|\\+|\\-).*$))|" +
-                      // early (pre-ea) "testing" builds of 11, 12, and 13 were also buggy
-                      // - https://bugs.openjdk.java.net/browse/JDK-8224829
-                      "(^(11|12|13).*-testing.*$)|" +
-                      // So far, all 13-ea builds (up to 13-ea-26) have been buggy
-                      // - https://bugs.openjdk.java.net/browse/JDK-8226338
-                      "(^13-ea.*$)"
-                      );
 }
diff --git a/solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java b/solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java
deleted file mode 100644
index 4e39954..0000000
--- a/solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java
+++ /dev/null
@@ -1,87 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.solr.util;
-
-import java.util.Arrays;
-import java.util.List;
-
-import org.apache.solr.SolrTestCase;
-
-public class TestSSLTestConfig extends SolrTestCase {
-
-  /** Sanity check that our JVM version parsing logic seems correct */
-  public void testIsOpenJdkJvmVersionKnownToHaveProblems() {
-    final List<String> rel_suffixes = Arrays.asList("", "+42");
-    final List<String> ea_suffixes = Arrays.asList("-ea", "-ea+42");
-    final List<String> suffixes = Arrays.asList("", "+42", "-ea", "-ea+42");
-
-    // as far as we know, any Java 8, 9 or 10 impl should be fine...
-    for (String base : Arrays.asList("1.8", "1.8.0", "1.8.1", 
-                                     "9", "9.0", "9.1", "9.0.0", "9.1.0", "9.1.1",
-                                     "10", "10.0", "10.1", "10.0.0", "10.1.0", "10.1.1")) {
-      for (String suffix : suffixes) {
-        final String v = base + suffix;
-        assertFalse(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
-      }
-    }
-
-    // Known Problems start with Java 11...
-
-    // java 11 releases below 11.0.3 were all bad...
-    for (String bad : Arrays.asList("11", "11.0", "11.0.1", "11.0.2")) {
-      for (String suffix : suffixes) {
-        final String v = bad + suffix;
-        assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
-      }
-    }
-    
-    // ...but 11.0.3 or higher should be ok.
-    for (String ok : Arrays.asList("11.0.3", "11.0.42", "11.1", "11.1.42")) {
-      for (String suffix : suffixes) {
-        final String v = ok + suffix;
-        assertFalse(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
-      }
-    }
-    
-    // As far as we know/hope, all "official" java 12 and higher impls should be fine...
-    for (String major : Arrays.asList("12", "13", "99")) {
-      for (String minor : Arrays.asList("", ".0", ".42", ".0.42")) {
-        for (String suffix : rel_suffixes) {
-          final String v = major + minor + suffix;
-          assertFalse(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
-        }
-      }
-    }
-
-    // ...but pre EA "testing" builds of 11, 12, and 13 are all definitely problematic...
-    for (String major : Arrays.asList("11", "12", "13")) {
-      for (String suffix : suffixes) {
-        final String v = major + "-testing" + suffix;
-        assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
-      }
-    }
-
-    // ...and all 13-ea builds (so far) have definitely been problematic.
-    for (String suffix : ea_suffixes) {
-      final String v = "13" + suffix;
-      assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
-    }
-    
-  }
-
-}