takari/aether-connector-okhttp

A repository connector implementation based on Square's OkHttp.

aether-connector-okhttp's Issues

Access to servers with TLSv1 is no longer supported

We failed to catch the issue earlier, as our tests reused locally cached data, but when a remote repository uses TLSv1, the connector fails to connect, as TLSv1 support was removed from OkHttp 3.13.0

https://ci-staging.eclipse.org/m2e/job/m2e-master/351/testReport/junit/org.eclipse.m2e.core.internal.project.registry/RegistryTest/testInvalidParent/

junit.framework.AssertionFailedError: expected:<Optional.empty> but was:<Optional[Marker [on: /invalidParent/pom.xml, id: 30, type: org.eclipse.m2e.core.maven2Problem.pomloading, attributes: [columnEnd: 9, columnStart: 1, lineNumber: 3, message: Project build error: Non-resolvable parent POM for org.eclipse.m2e.tests.invalidParent:invalidParent:1: Could not transfer artifact io.takari:takari:pom:28 from/to eclipse.maven.central.mirror (https://repo.eclipse.org/content/repositories/maven_central/): Server chose TLSv1, but that protocol version is not enabled or not supported by the client. and 'parent.relativePath' points at wrong local POM, severity: 2, transient: false], created: 5/28/19 6:43 PM]]> at org.eclipse.m2e.core.internal.project.registry.RegistryTest.testInvalidParent(RegistryTest.java:90)

Should be fixable by enabling the protocols on the client

OkHttpClient client = new OkHttpClient.Builder()
    .connectionSpecs(Arrays.asList(ConnectionSpec.COMPATIBLE_TLS))
    .build();

Race condition in AetherRepositoryConnector

Background

I'm investigating some jar file corruption occurring in my .m2 when using Eclipse's m2e plugin to import Maven projects.

Usually a jar file, that is larger than the original jar is saved in the .m2 folder. But sometimes the jar file is 0 bytes, other times, it smaller than the actual jar. Besides jars, pom.xmls are often getting corrupted too.
Checking the sha1 shows that the file is indeed corrupt.

This causing both Eclipse build, and later actual Maven build to fail for variety of reasons, like:

• Invalid ZIP file, when opening the jar
• Unparseable pom.xml, if the corruption happened in an XML
• NoSuchMethod, NoSuchClass exception, if there is no direct compile time reliance of these jars
• Not starting Spring Boot autoconfigurations
• and many others...

These are especially hard bugs to find.
More details in the Eclipse bugs:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=534228
https://bugs.eclipse.org/bugs/show_bug.cgi?id=514691

Repro

I have made an integration test that shows the the corruption reliably, you can find it here:
master...helospark:concurrency-test

This test simulates an Artifactory interaction with aether-connection, without actual remote calls.
Starts a few threads that are trying to download the same file.
It retries the test 10 times, to make sure the concurrency issues are shown.

Cause

The cause is the "resume download" feature built into the connector. Based on some debugging the following happens:

• Two threads enter, trying to download the same file
• First thread creates a tmp file (aether-...-in-progress) , and starts to write into the file
• Second thread find the tmp file, and "thinks" the previous download get interrupted, so it tries to download bytes after the file's current content (resuming the download)
• Now 2 threads are writing to the same file, usually result in a file larger than the original
• After the file is downloaded, checksum may fail (but it can happen, that the first threads's checksum check happens before the second thread flushes to the file).
• If the checksum behavior set to "warn", then the corrupt jar file just gets renamed to the final name
• If the checksum behavior set to "fail", then the corrupt jar does not get renamed, however during the next download, the corrupt tmp "in-progress" file is found, and the download will fail again, because this tmp file is never removed. Only manually clearing this from .m2 makes this download work.

Possible fixes

• Since most artifacts are relatively small, I don't really see the benefit of continuing a download. I think the simplest option would be to remove this feature. Or make it switchable.
• Another possibility is to not ignore checksum problems, instead redownload without the corrupt tmp file. Though this would create the problem, that multiple threads are retrying the same download.
• Tmp files should always be deleted at the end of the download or ignored during the next, whether successful or not, otherwise retrying the download will also fail every time.
• In-JVM solution to prevent the same file to download by multiple threads (eg. some locking mechanism that keeps tracks of which files are currently downloading, and prevents another thread entering with the same file).

Further considerations

• While removing the resume download feature locally, I have not been able to see any more jar corruption, there is still a small chance, that the FileProcessor.move method is not actually atomic. It uses the
  • File.renameTo to move the file, which this may not be atomic depending on the OS
  • If the renameTo fails, than it copies the content using FileOutputStream, which is by default not atomic. Though it always clears the destination file. I have not been able to get this to corrupt the file. If we want absolute security, we could use FileLocks here.

As long as the tmp files are always deleted (or not using during init), I don't think, we have to worry about these so much, because the likelihood of this causing an issue seems small, and a retry solves it.

aether-connector-okhttp 0.17.(5|6): frequent `Checksum validation failed` with `maven-deploy-plugin:2.8.2:deploy-file`

Hi,
After making one our projects use aether-connector-okhttp as a core extension, we realized mvn deploy:deploy-file started facing Checksum validation failed issues. By increasing the number of retries, the deployment eventually succeeds, which leads to suspect a race condition somewhere.

Here's an (anonymized) maven output:

...
[INFO] --- maven-deploy-plugin:2.8.2:deploy-file (default) @ my-project ---
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.zip
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.zip (1.2 MB at 191 kB/s)
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.pom
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.pom (455 B at 1.8 kB/s)
[INFO] Downloading from : https://artifactory.my-domain/my-path/my-project/maven-metadata.xml
[WARNING] Checksum validation failed: Checksum validation failed, expected faf34f17acf90e006c7078f43af8e16c454bb08a but is a101752c30e45f96aa41d6599629999676500721 from  for https://artifactory.my-domain/my-path/my-project/maven-metadata.xml
[WARNING] Could not transfer metadata my.group.id:my-project/maven-metadata.xml from/to my-repo-id (https://artifactory.my-domain): Checksum validation failed: Checksum validation failed, expected faf34f17acf90e006c7078f43af8e16c454bb08a but is a101752c30e45f96aa41d6599629999676500721
[WARNING] Encountered issue during deployment: Failed to retrieve remote metadata my.group.id:my-project/maven-metadata.xml: Could not transfer metadata my.group.id:my-project/maven-metadata.xml from/to my-repo-id (https://artifactory.my-domain): Checksum validation failed: Checksum validation failed, expected faf34f17acf90e006c7078f43af8e16c454bb08a but is a101752c30e45f96aa41d6599629999676500721
[INFO] Retrying deployment attempt 2 of 10
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.zip
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.zip (590 kB at 3.6 MB/s)
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.pom
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.pom (455 B at 3.6 kB/s)
[INFO] Downloading from : https://artifactory.my-domain/my-path/my-project/maven-metadata.xml
[WARNING] Checksum validation failed: Checksum validation failed, expected 735e6640ca292ee83b39e7604446dbe03df20202 but is faf34f17acf90e006c7078f43af8e16c454bb08a from  for https://artifactory.my-domain/my-path/my-project/maven-metadata.xml
[WARNING] Could not transfer metadata my.group.id:my-project/maven-metadata.xml from/to my-repo-id (https://artifactory.my-domain): Checksum validation failed: Checksum validation failed, expected 735e6640ca292ee83b39e7604446dbe03df20202 but is faf34f17acf90e006c7078f43af8e16c454bb08a
[WARNING] Encountered issue during deployment: Failed to retrieve remote metadata my.group.id:my-project/maven-metadata.xml: Could not transfer metadata my.group.id:my-project/maven-metadata.xml from/to my-repo-id (https://artifactory.my-domain): Checksum validation failed: Checksum validation failed, expected 735e6640ca292ee83b39e7604446dbe03df20202 but is faf34f17acf90e006c7078f43af8e16c454bb08a
[INFO] Retrying deployment attempt 3 of 10
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.zip
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.zip (590 kB at 4.7 MB/s)
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.pom
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/0.0.42/my-project-0.0.42.pom (455 B at 4.9 kB/s)
[INFO] Downloading from : https://artifactory.my-domain/my-path/my-project/maven-metadata.xml
[INFO] Downloaded from : https://artifactory.my-domain/my-path/my-project/maven-metadata.xml (22 kB at 1.5 MB/s)
[INFO] Uploading to : https://artifactory.my-domain/my-path/my-project/maven-metadata.xml
[INFO] Uploaded to : https://artifactory.my-domain/my-path/my-project/maven-metadata.xml (22 kB at 260 kB/s)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
...

Without retries, the build fails and the stack trace shows the ChecksumFailureException is raised at

aether-connector-okhttp/src/main/java/io/takari/aether/connector/AetherRepositoryConnector.java

Line 540 in 2040c61

throw new ChecksumFailureException(expected, actual);

Br,
Régis

OkHttpsWagon license text

The OkHttpsWagon class has both EPL and Apache v2.0 license text in the header. I assume the Apache license is from the original code (LightweightHttpsWagon). Is it really ok to relicense this under EPL then?

m2e deployments with multiple targets failing

See

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=566513
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=562847

Apparently the new multi thread put on io.takari.aether.connector.AetherRepositoryConnector is failing to handle 401 status on multiple tasks.

AetherClientProxy has no support for nonProxyHosts/noProxy

I'm working on m2e-core in a proxied environment. Some of the tests fail when a proxy is set in maven settings, but pass when no proxy is set. Of course, removing the proxy causes other tests to fail.

After debugging this, I found that the failing tests are failing because they are attempting to use proxy to reach localhost/127.0.0.1. I have nonProxyHosts set in my maven settings, but upon further debugging I noticed that nonProxyHosts is ignored in OkHttpWagon.openConnectionInternal because AetherClientProxy does not have any property for nonProxyHosts.

This seriously limits the usability of aether-connector-okhttp in proxied environments.

Preemtive authentication configuration

It will be nice to have a configuration to use preemtive authentication like in

https://maven.apache.org/guides/mini/guide-http-settings.html#Example:_Using_Preemptive_Authentication

Not only against proxies, but also against servers
Use case: Some Artifactory configurations force to use preemptive authentication.

okhttp 3.11.0 vulnerable to CVE-2018-20200

In order to mitigate this, upgrade okhttp dependency to latest release (3.14.1).
I intend to open a PR for this.
I'll build and run the tests to validate that the upgrade did not break anything.
Anything else needed for the PR to be accepted?

Archive this repository

Added README that explains that this project is superseded and direct users to latest resolver (w/ hint of plans for Resolver 1.10.0 to include HTTP/2 transports).

@jvanzyl @mosabua are you okay to archive this repository?

artifacts are downloaded serially in a single thread

Take a look at class AetherRepositoryConnector. For each artifact, there's a corresponding GetTask. And since there's a CountDownLatch, I believe the tasks are designed to run in parallel.
But, the code calls Runnable.run() directly, making it mono-threaded, see below:

aether-connector-okhttp/src/main/java/io/takari/aether/connector/AetherRepositoryConnector.java

Lines 299 to 306 in 2040c61

	for (ArtifactDownload download : artifactDownloads) {
	String resource = layout.getLocation(download.getArtifact(), false).getPath();
	GetTask<?> task = new GetTask<ArtifactTransfer>(resource, download.isExistenceCheck() ? null : download.getFile(), download.getChecksumPolicy(), latch, download, ARTIFACT);
	tasks.add(task);
	task.run();
	}
	await(latch);

RejectedExecutionException in m2e

The issue has been introduced with 32ce3f8
It can be reproduced by importing spring-petclinic into an empty maven workspace.
See eclipse-jdtls/eclipse.jdt.ls#1369 and https://ci.eclipse.org/ls/job/jdt-ls-pr/1903/