Do not eagerly resolve Maven repositories, to prevent SSLHandshakeException stacktrace from maven.java.net
#4101
Comments
|
Just noting it here also, but there's a thread in Slack mentioning the same issue. Could potentially catch the exception from an OpenRewrite standpoint, but I'm also guessing that this likely appears directly from Maven as well for the same reasons. |
|
@shanman190 no, I don't see anything from Maven. Only OpenRewrite is complaining. |
|
That's interesting. @sambsnyd, do you have any opinions on how you'd like to see this handled? |
|
I wonder if OpenRewrite selects the repositories the same way as Maven. I wasn't able to find where these repositories came from exactly but I wouldn't expect them to be used if things are in Maven Central. |
|
I think it could be appropriate in rewrite-maven-plugin to check some of the common SSL-skipping parameters used in Maven builds and provide an appropriate http client in the execution context |
|
I’m not sure the problem is due to that really. For the other project, it was contacting another server that wasn’t responding at all. I think the problem is that you shouldn’t contact these servers at all in the first place. |
|
To further clarify: I'm not sure adding the Maven Central repository last is actually what Maven does by default. At least, not compared to repositories declared in dependencies of the project. Another option could be to not report any error when it's impossible to download the artifact. I mean I could see how you could maybe pile up the download errors and only log them if in the end we can't download the artifact from any repository? But I think getting a clear picture of the correct ordering is important, because in the case I had where some artifact somewhere was referencing a repository that was non-existent and timing out, it would make my build a lot slower if we try to resolve all artifacts with this repository first. |
I want to reiterate this. IIRC, java.net should not be used for anything at all anymore, really, see the notice at[1]. I assume they're keeping up their Nexus instance so as not to break anyone's build if you depend on artifacts from projects that for some reason haven't moved on to other maven repositories (like central). Unfortunately their SSL certificate has simply expired. I have personally emailed both the domain owner as well as the mail-address mentioned on [1]. Perhaps it would be for the better if they just opted to shut down said Nexus. |
|
It is used because some artifacts somewhere in the dependency tree has it. You can't drop it from existing artifacts. And I know that when resolving the dependencies of artifact1, if artifact1 has java.net as a repository, you might contact it... but my understanding was that we would hit Maven Central first and not last. What needs fixing is that we should either not contact it at all if in Maven Central as I think Maven resolves Maven Central by default first or we should ignore the error if we can find it in a later repositories. |
|
https://maven.apache.org/guides/mini/guide-multiple-repositories.html#repository-order It's possible that internal Maven code is out of sync with this, but the documentation kinda makes it seem like if Maven settings aren't used and some repository that contains the artifact isn't explicitly defined somewhere prior, then it should hit java.net (undesirably) by the order of the rules there. It also makes it seem like the default super pom comes after any local or parent poms, but before dependency poms. So it does seem like there may be some incorrect ordering within rewrite-maven as well. |
|
It is NOT ONLY a warning: It stops all processing when this error occures and nothing is transformed. Unusable stuff at all. |
I think you may have misunderstood. What I meant is that a conscious effort should be made to update dependencies such that we don't get any kind of (not even transitive) reference to the old maven.java.net repository. If we can't do that (because a dependency is not replaceable for instance), the effort should go into assisting the dependency to move on etc.
Ignoring the error if it's found in later repositories is what maven does I believe. It's an acceptable quick solution I think. If the logic used here comes from the maven code, perhaps the effort should be put into improving maven. FWIW: one of the things that has annoyed me in maven is that if in a dependency a specific repo is mentioned, maven contacts it for all dependencies it tries to resolve after, including local dependencies in multi-module projects and therefore leaking miniscule bits of information of your project to some repository. Ignore this aside as it's not relevant for this issue. |
|
@timtebeek and @sambsnyd this one feels pretty important. Do we simply need a carve out exclusion to never contact java.net? On the other point, I am pretty confident Maven Central is added last deep in the Maven code. |
|
That's not only java.net. I had the issues with some other repositories that were declared in some dependencies. And I don't recall having seen failures of artifacts being downloaded from these repositories first when using regular Maven so I'm not entirely sure the logic in OpenRewrite is right. |
|
I've had numerous issue with openrewrite because its clearly not doing things by the book. Pls get a Maven maintainer to look at how this project is working with Maven. |
|
@delanym FWIW, that is not my personal experience. Anyway, there are more pleasant ways to convey this message. |
|
I tried again on two of my projects to gather a bit more information. For https://github.com/quarkusio/quarkus-github-bot, I get the warning twice:
Unfortunately, there are no details about the requests in the warnings but what I find odd is that both are for snapshots repositories and the only snapshot version in this project is the version of the project itself (and I wouldn't expect it to try to download the modules from the project). I built the projects (regular
as the two contacted repositories. From what I can see the I have the same experience with https://github.com/quarkiverse/quarkus-github-app/ but I only have one warning there:
I remember having seen another project with I'm sorry, I don't have the time to dig more at the moment but I hope this could be helpful. |
|
Repositories are defined in the poms included in a JAR, like woodstox-core-6.5.1/META-INF/maven/com.sun.xml.bind.jaxb/isorelax/pom.xml <repositories>
<repository>
<id>releases.java.net</id>
<url>https://maven.java.net/content/repositories/releases/</url>
<layout>default</layout>
</repository>
<repository>
<id>jvnet-nexus-staging</id>
<url>https://maven.java.net/content/repositories/staging/</url>
<layout>default</layout>
</repository>
<repository>
<id>eclipse</id>
<url>http://download.eclipse.org/rt/eclipselink/maven.repo</url>
<layout>default</layout>
</repository>
</repositories>Why openrewrite would read this I don't know I'm not a Maven expert, but neither should you have to be. There's no shame in asking for help. We cant all be experts in every field, and Maven is a complex beast all by itself. |
|
@delanym I'm not part of this project so I only speak for myself but really, again, you can say the same things without being borderline insulting. Feedback is useful but there's really no need to be hostile. |
|
No hostility intended |
|
👋 I have the same issue as well. Is there a known workaround? This is blocking |
|
I have the same issue. It leads to a |
timtebeek
added
question
|
So we just did a new release yesterday that folks here might want to try out: https://github.com/openrewrite/rewrite-recipe-bom/releases/tag/v2.10.0 I've done a quick recipe run locally using this latest version and I had no issue getting recipe results from OrderImports against the above mentioned https://github.com/quarkiverse/quarkus-github-app/ just now This despite this warning that we surface, but does not break running recipes |
|
You are right. The execution-breaking error I referred to comes from dependencies using the |
|
Thanks for chiming in with that insight; good that recipe runs are at least not affected. That brings us back to the message Guillaume started with: that it's probably best to not show the full stacktrace if this is not affecting recipe runs in practice. With this PR we already have some work in progress that touches upon some of the same areas I propose we wait for the above PR to be merged and then reevaluate if we still have to change the reported stacktrace here. |
|
I have one example where changes are applied but the stacktrace displayed Environment Adding the |
|
is there any way to work around the issue in the meantime? |
Do you have a reproducer? That's been the main hold up to work on this. |
|
I have been working on an update to the |
|
Sorry, time is scarce on my side and I wasn't able to provide feedback here earlier. Here is a simple reproducer with a simple project, a simple recipe and using the OpenRewrite Maven plugin directly: You will get this warning during the execution: When building the project with Maven, even with an empty Maven repo, you don't get this warning so, as I explained a bit earlier, there's something incorrect in how the repositories are resolved. |
|
Thanks for the clear steps @gsmet ! Indeed I see that warning logged before making the correct changes with that recipe. In my debugger I see that Which itself has the following dependencies (among others). Zooming on on that We can see that has a parent of
<repositories>
<repository>
<id>jvnet-nexus-snapshots</id>
<name>Java.net Nexus Snapshots Repository</name>
<url>https://maven.java.net/content/repositories/snapshots</url>
<releases>
<enabled>false</enabled>
<updatePolicy>never</updatePolicy>
</releases>
<snapshots>
<enabled>true</enabled>
<updatePolicy>never</updatePolicy>
</snapshots>
</repository>
</repositories>That at least gives us a path of where the look up is coming from; next up a way to either prevent that call, or silence the reported issue somewhat. |
|
Yeah, FWIW, it's not the only problematic artifact. Old legacy artifacts often point to a repository that doesn't exist anymore. In Quarkus, we are using jakarta.annotation but... we rely on the Maven artifacts here that require the But as you can see from the various reports you had, it happens quite a lot in the wild as not all the reports here are related to Quarkus. |
|
Hmm; Closes this a little early through my choice of words on openrewrite/rewrite-maven-plugin#811 , but in short what I've done there hide the stack trace by default, as so far no one has come up with a reproducer that actually blocks recipes from making changes. This message only started to appear for folks after we added the logging (only) in the plugin: I'll continue to explore if we can prevent contacting that repository where it's not needed to hopefully fully get rid of the warning message as well. |
|
Folks looking to explore this issue in more depth can do so using this unit test. @Test
@Issue("https://github.com/openrewrite/rewrite/issues/4101")
void mavenJavaNetFailedSSLNeedNotBeContactedForJavaxAnnotationApi() throws MavenDownloadingException {
InMemoryExecutionContext ctx = new InMemoryExecutionContext();
AtomicBoolean attempted = new AtomicBoolean(false);
MavenExecutionContextView.view(ctx).setResolutionListener(new ResolutionEventListener() {
@Override
public void repositoryAccessFailed(String uri, Throwable e) {
attempted.set(true);
}
});
MavenPomDownloader downloader = new MavenPomDownloader(ctx);
// This has a parent of `net.java:jvnet-parent:3`, which itself has a snapshot repository for maven.java.net
GroupArtifactVersion annotationGav = new GroupArtifactVersion("javax.annotation", "javax.annotation-api", "1.3.2");
Pom downloadedPom = downloader.download(annotationGav, null, null, singletonList(MAVEN_CENTRAL));
ResolvedPom resolvedPom = downloadedPom.resolve(List.of(), downloader, ctx);
assertThat(resolvedPom).isNotNull();
assertThat(attempted.get())
.withFailMessage("maven.java.net was contacted, but without an immediate need to do so")
.isFalse();
}You'll notice we log the |
|
Total crap, Oracle is too stupid to configure the certificate chain sent to the client. Just hit this as well: Similar: https://bugzilla.mozilla.org/show_bug.cgi?id=1763708 Here is the breakdown: "DigiCert TLS RSA SHA256 2020 CA1" is an intermediate, but neither part of Mozilla NSS, nor OpenJDK cacerts. The missed to send the root certificate DN as well: Quoting from https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslcertificatefile: Ugly solution:
|
|
Thanks for chiming in here with those details @michael-o ! Good to know it's not just us, and caused by a certificate issue that we warn about, but continue in spite of to make the required changes. |
Hmmm. Really that's not the root of the issue. We had this particular issue with:
Some very old dependencies registers repositories that are not really useful and are not actually used anymore. From my experiments, in a normal Maven run, these repositories are not even contacted. You have nothing in your logs about them, even if you drop your To fix the issue, OpenRewrite would need to resolve the repositories more like Maven does (easier said than done, I agree). |
Maybe true, but I don't know what maven.java.net carries, but still there is no justification to make TLS setup right. You see how much trouble it has caused. |
|
Would like to draw attention to following things that were added to Maven not so long time ago:
Basically, in ideal case:
In Maven, that repo is not even touched, as can be seen: Reason is simple:
|
timtebeek
changed the title
SSLHandshakeException stacktrace from maven.java.net not actionable, possibly should not have been attemptedSSLHandshakeException stacktrace from maven.java.net
|
Thanks for that added context! In our case we have our own implementation to resolve dependencies recursively, and the repositories those need, which funnels through this method to eagerly contact repositories. The reasons for that eager connect are internal: we store which repositories are reachable as part of our (at times serialized) model, such that we know which repositories to (not) contact when folks might want to add a new dependency, or need to resolve the latest versions. I agree that it's perhaps good to revise that handling such that we do not contact repositories that aren't needed immediately; although in the short term we've already reduced the error visibility through |
|
@delabassee Can you reach out to someone internally to get the TLS chain setup fixed? |
|
The issue with maven.java.net has been fixed. Thanks to @brianf |
|
I don't think this is a bug. It only occurs when the website's certificate is invalid. You can choose to add |
I have noticed several warnings lately when updating various projects related to repositories not being available or not up to date.
As far as I can see, it's usually dependencies coming with exotic repositories declared and it seems that OpenRewrite resolves all of them (not sure if it's new but I noticed it only lately).
It generates big warnings in the output such as:
(This is just an example, I had another issue with some dependency in the classpath registering central.maven.org as a repository...)
Given there's nothing we can do about it as they come from external dependencies, I wonder if this should be debug logging instead?
At least for repositories coming from dependencies and not from the current project.
I have seen this lately in two different projects (and two different errors) so I suspect this might be more common than we think.
Thoughts?
The text was updated successfully, but these errors were encountered: