Skip to content

Commit

Permalink
Update CI config. (#1235)
Browse files Browse the repository at this point in the history
Travis and lgtm.com configs are obsolete.
Fix tests for Java 17+
Simplify YAML for matrix testing, and add macos-13 which should give coverage on x86 as well as ARM.
Reduce log level for session errors.
Add timeouts.
Align java test versions to LTS.
Refactor TestUtils -> Platform shims.
  • Loading branch information
prbprbprb authored Oct 14, 2024
1 parent d8be94d commit 90f0a86
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 202 deletions.
45 changes: 28 additions & 17 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ jobs:

steps:
- name: Set up JDK 11 for toolchains
uses: actions/setup-java@v1.4.3
uses: actions/setup-java@v4
with:
distribution: 'zulu'
java-version: 11

- name: Set runner-specific environment variables
Expand All @@ -59,7 +60,7 @@ jobs:
echo "SDKMANAGER=${{ runner.temp }}/android-sdk/cmdline-tools/bin/sdkmanager" >> $GITHUB_ENV
echo "M2_REPO=${{ runner.temp }}/m2" >> $GITHUB_ENV
- uses: actions/checkout@v2
- uses: actions/checkout@v4

- name: Setup Linux environment
if: runner.os == 'Linux'
Expand All @@ -83,9 +84,9 @@ jobs:
brew install ninja || echo update failed
- name: install Go
uses: actions/setup-go@v1
uses: actions/setup-go@v5
with:
go-version: '1.19.3'
go-version: '1.20'

- name: Setup Windows environment
if: runner.os == 'Windows'
Expand Down Expand Up @@ -202,7 +203,7 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4

- name: Set runner-specific environment variables
shell: bash
Expand Down Expand Up @@ -266,22 +267,31 @@ jobs:
strategy:
fail-fast: false
matrix:
platform: [ubuntu-latest, macos-latest, windows-latest]
java: [8, 9, 11]
platform: [ubuntu-latest, macos-13, macos-latest, windows-latest]
java: [8, 11, 17, 21]
dist: ['temurin', 'zulu']
include:
- java: 8
suite_class: "org.conscrypt.Conscrypt(OpenJdk)?Suite"
- java: 9
suite_class: "org.conscrypt.Conscrypt(OpenJdk)?Suite"
- java: 11
suite_class: "org.conscrypt.Conscrypt(OpenJdk)?Suite"
- platform: ubuntu-latest
separator: ':'
- platform: macos-latest
separator: ':'
- platform: macos-13
separator: ':'
- platform: windows-latest
separator: ';'
exclude: # Not available on Github runners
- platform: macos-latest
java: 8
dist: 'temurin'


runs-on: ${{ matrix.platform }}

steps:
- name: Set up Java
uses: actions/setup-java@v1
uses: actions/setup-java@v4
with:
distribution: ${{ matrix.dist }}
java-version: ${{ matrix.java }}

- name: Download UberJAR
Expand All @@ -298,19 +308,20 @@ jobs:

- name: Download JUnit runner
shell: bash
run: mvn org.apache.maven.plugins:maven-dependency-plugin:3.1.2:copy -Dartifact=org.junit.platform:junit-platform-console-standalone:1.6.2 -DoutputDirectory=. -Dmdep.stripVersion=true
run: mvn org.apache.maven.plugins:maven-dependency-plugin:3.8.0:copy -Dartifact=org.junit.platform:junit-platform-console-standalone:1.11.2 -DoutputDirectory=. -Dmdep.stripVersion=true

- name: Run JUnit tests
timeout-minutes: 15
shell: bash
run: |
DIR="$(find m2/org/conscrypt/conscrypt-openjdk-uber -maxdepth 1 -mindepth 1 -type d -print)"
VERSION="${DIR##*/}"
TESTJAR="$(find testjar -name '*-tests.jar')"
java -jar junit-platform-console-standalone.jar -cp "$DIR/conscrypt-openjdk-uber-$VERSION.jar:$TESTJAR" -n='${{ matrix.suite_class }}' --scan-classpath --reports-dir=results --fail-if-no-tests
java -jar junit-platform-console-standalone.jar execute -cp "$DIR/conscrypt-openjdk-uber-$VERSION.jar${{ matrix.separator }}$TESTJAR" -n='org.conscrypt.ConscryptOpenJdkSuite' --scan-classpath --reports-dir=results --fail-if-no-tests
- name: Archive test results
if: ${{ always() }}
uses: actions/upload-artifact@v4
with:
name: test-results-${{ matrix.platform }}-${{ matrix.java }}
name: test-results-${{ matrix.platform }}-${{ matrix.java }}-${{ matrix.dist }}
path: results
21 changes: 0 additions & 21 deletions .lgtm.yml

This file was deleted.

117 changes: 0 additions & 117 deletions .travis.yml

This file was deleted.

4 changes: 2 additions & 2 deletions common/src/main/java/org/conscrypt/NativeSslSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ byte[] toBytes() {
return baos.toByteArray();
} catch (IOException e) {
// TODO(nathanmittler): Better error handling?
logger.log(Level.WARNING, "Failed to convert saved SSL Session: ", e);
logger.log(Level.FINE, "Failed to convert saved SSL Session: ", e);
return null;
} catch (CertificateEncodingException e) {
log(e);
Expand Down Expand Up @@ -464,7 +464,7 @@ public int getApplicationBufferSize() {

private static void log(Throwable t) {
// TODO(nathanmittler): Better error handling?
logger.log(Level.INFO, "Error inflating SSL session: {0}",
logger.log(Level.FINE, "Error inflating SSL session: {0}",
(t.getMessage() != null ? t.getMessage() : t.getClass().getName()));
}

Expand Down
27 changes: 27 additions & 0 deletions common/src/test/java/org/conscrypt/javax/crypto/CipherTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.security.KeyPairGenerator;
import java.security.PrivateKey;
import java.security.Provider;
import java.security.ProviderException;
import java.security.PublicKey;
import java.security.SecureRandom;
import java.security.Security;
Expand Down Expand Up @@ -3907,6 +3908,14 @@ private void checkCipher(CipherTestParam p, String provider) throws Exception {
if (!isAEAD(p.transformation)) {
throw maybe;
}
} catch (ProviderException maybe) {
boolean isShortBufferException
= maybe.getCause() instanceof ShortBufferException;
if (!isAEAD(p.transformation)
|| !isBuggyProvider(provider)
|| !isShortBufferException) {
throw maybe;
}
}
try {
c.update(new byte[0]);
Expand All @@ -3920,6 +3929,14 @@ private void checkCipher(CipherTestParam p, String provider) throws Exception {
if (!isAEAD(p.transformation)) {
throw maybe;
}
} catch (ProviderException maybe) {
boolean isShortBufferException
= maybe.getCause() instanceof ShortBufferException;
if (!isAEAD(p.transformation)
|| !isBuggyProvider(provider)
|| !isShortBufferException) {
throw maybe;
}
}
} else {
throw new AssertionError("Define your behavior here for " + provider);
Expand Down Expand Up @@ -4014,6 +4031,13 @@ private void checkCipher(CipherTestParam p, String provider) throws Exception {
}
}

// SunJCE has known issues between 17 and 21
private boolean isBuggyProvider(String providerName) {
return providerName.equals("SunJCE")
&& TestUtils.isJavaVersion(17)
&& !TestUtils.isJavaVersion(21);
}

/**
* Gets the Cipher transformation with the same algorithm and mode as the provided one but
* which uses no padding.
Expand Down Expand Up @@ -4514,6 +4538,9 @@ public void testRC4_MultipleKeySizes() throws Exception {
public void testAES_keyConstrained() throws Exception {
Provider[] providers = Security.getProviders();
for (Provider p : providers) {
if (isBuggyProvider(p.getName())) {
continue;
}
for (Provider.Service s : p.getServices()) {
if (s.getType().equals("Cipher")) {
if (s.getAlgorithm().startsWith("AES_128/")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,7 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import tests.net.DelegatingSSLSocketFactory;
Expand Down Expand Up @@ -380,8 +378,6 @@ public void handshakeCompleted(HandshakeCompletedEvent event) {
String cipherSuite = event.getCipherSuite();
Certificate[] localCertificates = event.getLocalCertificates();
Certificate[] peerCertificates = event.getPeerCertificates();
javax.security.cert.X509Certificate[] peerCertificateChain =
event.getPeerCertificateChain();
Principal peerPrincipal = event.getPeerPrincipal();
Principal localPrincipal = event.getLocalPrincipal();
socket = event.getSocket();
Expand Down Expand Up @@ -410,17 +406,21 @@ public void handshakeCompleted(HandshakeCompletedEvent event) {
.assertServerCertificateChain(c.clientTrustManager, peerCertificates);
TestSSLContext
.assertCertificateInKeyStore(peerCertificates[0], c.serverKeyStore);
assertNotNull(peerCertificateChain);
TestKeyStore.assertChainLength(peerCertificateChain);
assertNotNull(peerCertificateChain[0]);
TestSSLContext.assertCertificateInKeyStore(
peerCertificateChain[0].getSubjectDN(), c.serverKeyStore);
assertNotNull(peerPrincipal);
TestSSLContext.assertCertificateInKeyStore(peerPrincipal, c.serverKeyStore);
assertNull(localPrincipal);
assertNotNull(socket);
assertSame(client, socket);
assertNull(socket.getHandshakeSession());
if (TestUtils.isJavaxCertificateSupported()) {
javax.security.cert.X509Certificate[] peerCertificateChain =
event.getPeerCertificateChain();
assertNotNull(peerCertificateChain);
TestKeyStore.assertChainLength(peerCertificateChain);
assertNotNull(peerCertificateChain[0]);
TestSSLContext.assertCertificateInKeyStore(
peerCertificateChain[0].getSubjectDN(), c.serverKeyStore);
}
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
Expand Down
16 changes: 15 additions & 1 deletion openjdk/src/main/java/org/conscrypt/Platform.java
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,22 @@ public static String getOriginalHostNameFromInetAddress(InetAddress addr) {
return originalHostName;
} catch (InvocationTargetException e) {
throw new RuntimeException("Failed to get originalHostName", e);
} catch (ClassNotFoundException | IllegalAccessException | NoSuchMethodException ignore) {
} catch (ClassNotFoundException | IllegalAccessException | NoSuchMethodException ignored) {
// passthrough and return addr.getHostAddress()
} catch (Exception maybeIgnored) {
if (!maybeIgnored.getClass().getSimpleName().equals("InaccessibleObjectException")) {
throw new RuntimeException("Failed to get originalHostName", maybeIgnored);
}
// Java versions which prevent reflection to get the original hostname.
// Ugly workaround is parse it from toString(), which uses holder.hostname rather
// than holder.originalHostName. But in Java versions up to 21 at least and in the way
// used by Conscrypt, hostname always equals originalHostname.
String representation = addr.toString();
int slash = representation.indexOf('/');
if (slash != -1) {
return representation.substring(0, slash);
}
// Give up and return the IP
}

return addr.getHostAddress();
Expand Down
Loading

0 comments on commit 90f0a86

Please sign in to comment.