[ https://issues.apache.org/jira/browse/MRESOLVER-308?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17651843#comment-17651843 ]
ASF GitHub Bot commented on MRESOLVER-308: ------------------------------------------ olamy commented on code in PR #231: URL: https://github.com/apache/maven-resolver/pull/231#discussion_r1056884304 ########## maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java: ########## @@ -0,0 +1,539 @@ +package org.eclipse.aether.transport.jetty; + +/* + * 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. + */ + +import javax.net.ssl.SSLContext; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.eclipse.aether.ConfigurationProperties; +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.repository.AuthenticationContext; +import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.spi.connector.transport.AbstractTransporter; +import org.eclipse.aether.spi.connector.transport.GetTask; +import org.eclipse.aether.spi.connector.transport.PeekTask; +import org.eclipse.aether.spi.connector.transport.PutTask; +import org.eclipse.aether.spi.connector.transport.TransportTask; +import org.eclipse.aether.transfer.NoTransporterException; +import org.eclipse.aether.util.ConfigUtils; +import org.eclipse.aether.util.FileUtils; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.HttpProxy; +import org.eclipse.jetty.client.api.Authentication; +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.client.dynamic.HttpClientTransportDynamic; +import org.eclipse.jetty.client.http.HttpClientConnectionFactory; +import org.eclipse.jetty.client.util.BasicAuthentication; +import org.eclipse.jetty.client.util.InputStreamResponseListener; +import org.eclipse.jetty.client.util.PathRequestContent; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http2.client.HTTP2Client; +import org.eclipse.jetty.http2.client.http.ClientConnectionFactoryOverHTTP2; +import org.eclipse.jetty.io.ClientConnector; +import org.eclipse.jetty.util.ssl.SslContextFactory; + +/** + * A transporter for HTTP/HTTPS. + */ +final class JettyTransporter + extends AbstractTransporter +{ + private static final int MULTIPLE_CHOICES = 300; + + private static final int NOT_FOUND = 404; + + private static final int PRECONDITION_FAILED = 412; + + private static final long MODIFICATION_THRESHOLD = 60L * 1000L; + + private static final String ACCEPT_ENCODING = "Accept-Encoding"; + + private static final String CONTENT_LENGTH = "Content-Length"; + + private static final String CONTENT_RANGE = "Content-Range"; + + private static final String IF_UNMODIFIED_SINCE = "If-Unmodified-Since"; + + private static final String RANGE = "Range"; + + private static final String USER_AGENT = "User-Agent"; + + private static final Pattern CONTENT_RANGE_PATTERN = + Pattern.compile( "\\s*bytes\\s+([0-9]+)\\s*-\\s*([0-9]+)\\s*/.*" ); + + private final URI baseUri; + + private final HttpClient client; + + private final int requestTimeout; + + private final Map<String, String> headers; + + JettyTransporter( RepositorySystemSession session, RemoteRepository repository ) throws NoTransporterException + { + try + { + URI uri = new URI( repository.getUrl() ).parseServerAuthority(); + if ( uri.isOpaque() ) + { + throw new URISyntaxException( repository.getUrl(), "URL must not be opaque" ); + } + if ( uri.getRawFragment() != null || uri.getRawQuery() != null ) + { + throw new URISyntaxException( repository.getUrl(), "URL must not have fragment or query" ); + } + String path = uri.getPath(); + if ( path == null ) + { + path = "/"; + } + if ( !path.startsWith( "/" ) ) + { + path = "/" + path; + } + if ( !path.endsWith( "/" ) ) + { + path = path + "/"; + } + this.baseUri = URI.create( uri.getScheme() + "://" + uri.getRawAuthority() + path ); + } + catch ( URISyntaxException e ) + { + throw new NoTransporterException( repository, e.getMessage(), e ); + } + + HashMap<String, String> headers = new HashMap<>(); + String userAgent = ConfigUtils.getString( session, + ConfigurationProperties.DEFAULT_USER_AGENT, + ConfigurationProperties.USER_AGENT ); + if ( userAgent != null ) + { + headers.put( USER_AGENT, userAgent ); + } + @SuppressWarnings( "unchecked" ) + Map<Object, Object> configuredHeaders = + (Map<Object, Object>) ConfigUtils.getMap( session, Collections.emptyMap(), + ConfigurationProperties.HTTP_HEADERS + "." + repository.getId(), + ConfigurationProperties.HTTP_HEADERS ); + if ( configuredHeaders != null ) + { + configuredHeaders.forEach( + ( k, v ) -> headers.put( String.valueOf( k ), v != null ? String.valueOf( v ) : null ) ); + } + + this.headers = headers; + + this.requestTimeout = ConfigUtils.getInteger( session, + ConfigurationProperties.DEFAULT_REQUEST_TIMEOUT, + ConfigurationProperties.REQUEST_TIMEOUT + "." + repository.getId(), + ConfigurationProperties.REQUEST_TIMEOUT ); + + this.client = getOrCreateClient( session, repository ); + } + + private URI resolve( TransportTask task ) + { + return baseUri.resolve( task.getLocation() ); + } + + @Override + public int classify( Throwable error ) + { + if ( error instanceof JettyException + && ( (JettyException) error ).getStatusCode() == NOT_FOUND ) + { + return ERROR_NOT_FOUND; + } + return ERROR_OTHER; + } + + @Override + protected void implPeek( PeekTask task ) + throws Exception + { + Request request = client.newRequest( resolve( task ) ) + .timeout( requestTimeout, TimeUnit.MILLISECONDS ) + .method( "HEAD" ); + request.headers( m -> headers.forEach( m::add ) ); + Response response = request.send(); + if ( response.getStatus() >= MULTIPLE_CHOICES ) + { + throw new JettyException( response.getStatus() ); + } + } + + @Override + protected void implGet( GetTask task ) + throws Exception + { + boolean resume = task.getResumeOffset() > 0L && task.getDataFile() != null; + Response response; + InputStreamResponseListener listener; + + while ( true ) + { + Request request = client.newRequest( resolve( task ) ) + .timeout( requestTimeout, TimeUnit.MILLISECONDS ) + .method( "GET" ); + request.headers( m -> headers.forEach( m::add ) ); + + if ( resume ) + { + long resumeOffset = task.getResumeOffset(); + request.headers( h -> + { + h.add( RANGE, "bytes=" + resumeOffset + '-' ); + h.addDateField( IF_UNMODIFIED_SINCE, + task.getDataFile().lastModified() - MODIFICATION_THRESHOLD ); + h.remove( HttpHeader.ACCEPT_ENCODING ); + h.add( ACCEPT_ENCODING, "identity" ); + } ); + } + + listener = new InputStreamResponseListener(); + request.send( listener ); + try + { + response = listener.get( requestTimeout, TimeUnit.MILLISECONDS ); + } + catch ( ExecutionException e ) + { + Throwable t = e.getCause(); + if ( t instanceof Exception ) + { + throw (Exception) t; + } + else + { + throw new RuntimeException( t ); + } + } + if ( response.getStatus() >= MULTIPLE_CHOICES ) + { + if ( resume && response.getStatus() == PRECONDITION_FAILED ) + { + resume = false; + continue; + } + throw new JettyException( response.getStatus() ); + } + break; + } + + long offset = 0L, length = response.getHeaders().getLongField( CONTENT_LENGTH ); + if ( resume ) + { + String range = response.getHeaders().get( CONTENT_RANGE ); + if ( range != null ) + { + Matcher m = CONTENT_RANGE_PATTERN.matcher( range ); + if ( !m.matches() ) + { + throw new IOException( "Invalid Content-Range header for partial download: " + range ); + } + offset = Long.parseLong( m.group( 1 ) ); + length = Long.parseLong( m.group( 2 ) ) + 1L; + if ( offset < 0L || offset >= length || ( offset > 0L && offset != task.getResumeOffset() ) ) + { + throw new IOException( "Invalid Content-Range header for partial download from offset " + + task.getResumeOffset() + ": " + range ); + } + } + } + + final boolean downloadResumed = offset > 0L; + final File dataFile = task.getDataFile(); + if ( dataFile == null ) + { + try ( InputStream is = listener.getInputStream() ) + { + utilGet( task, is, true, length, downloadResumed ); + } + } + else + { + try ( FileUtils.CollocatedTempFile tempFile = FileUtils.newTempFile( dataFile.toPath() ) ) + { + task.setDataFile( tempFile.getPath().toFile(), downloadResumed ); + if ( downloadResumed && Files.isRegularFile( dataFile.toPath() ) ) + { + try ( InputStream inputStream = Files.newInputStream( dataFile.toPath() ) ) + { + Files.copy( inputStream, tempFile.getPath(), StandardCopyOption.REPLACE_EXISTING ); + } + } + try ( InputStream is = listener.getInputStream() ) + { + utilGet( task, is, true, length, downloadResumed ); + } + tempFile.move(); + } + finally + { + task.setDataFile( dataFile ); + } + } + Map<String, String> checksums = extractXChecksums( response ); + if ( checksums != null ) + { + checksums.forEach( task::setChecksum ); + return; + } + checksums = extractNexus2Checksums( response ); + if ( checksums != null ) + { + checksums.forEach( task::setChecksum ); + } + } + + private static Map<String, String> extractXChecksums( Response response ) + { + String value; + HashMap<String, String> result = new HashMap<>(); + // Central style: x-checksum-sha1: c74edb60ca2a0b57ef88d9a7da28f591e3d4ce7b + value = response.getHeaders().get( "x-checksum-sha1" ); + if ( value != null ) + { + result.put( "SHA-1", value ); + } + // Central style: x-checksum-md5: 9ad0d8e3482767c122e85f83567b8ce6 + value = response.getHeaders().get( "x-checksum-md5" ); + if ( value != null ) + { + result.put( "MD5", value ); + } + if ( !result.isEmpty() ) + { + return result; + } + // Google style: x-goog-meta-checksum-sha1: c74edb60ca2a0b57ef88d9a7da28f591e3d4ce7b + value = response.getHeaders().get( "x-goog-meta-checksum-sha1" ); + if ( value != null ) + { + result.put( "SHA-1", value ); + } + // Central style: x-goog-meta-checksum-sha1: 9ad0d8e3482767c122e85f83567b8ce6 + value = response.getHeaders().get( "x-goog-meta-checksum-md5" ); + if ( value != null ) + { + result.put( "MD5", value ); + } + + return result.isEmpty() ? null : result; + } + + private static Map<String, String> extractNexus2Checksums( Response response ) + { + // Nexus-style, ETag: "{SHA1{d40d68ba1f88d8e9b0040f175a6ff41928abd5e7}}" + String etag = response.getHeaders().get( "ETag" ); + if ( etag != null ) + { + int start = etag.indexOf( "SHA1{" ), end = etag.indexOf( "}", start + 5 ); + if ( start >= 0 && end > start ) + { + return Collections.singletonMap( "SHA-1", etag.substring( start + 5, end ) ); + } + } + return null; + } + + @Override + protected void implPut( PutTask task ) + throws Exception + { + Request request = client.newRequest( resolve( task ) ).method( "PUT" ) + .timeout( requestTimeout, TimeUnit.MILLISECONDS ); + request.headers( m -> headers.forEach( m::add ) ); + try ( FileUtils.TempFile tempFile = FileUtils.newTempFile() ) + { + utilPut( task, Files.newOutputStream( tempFile.getPath() ), true ); + request.body( new PathRequestContent( tempFile.getPath() ) ); + + Response response; + try + { + response = request.send(); + } + catch ( ExecutionException e ) + { + Throwable t = e.getCause(); + if ( t instanceof Exception ) + { + throw (Exception) t; + } + else + { + throw new RuntimeException( t ); + } + } + if ( response.getStatus() >= MULTIPLE_CHOICES ) + { + throw new JettyException( response.getStatus() ); + } + } + } + + @Override + protected void implClose() + { + // noop + } + + /** + * Visible for testing. + */ + static final String JETTY_INSTANCE_KEY_PREFIX = JettyTransporterFactory.class.getName() + ".jetty."; + + @SuppressWarnings( "checkstyle:methodlength" ) + private static HttpClient getOrCreateClient( RepositorySystemSession session, RemoteRepository repository ) + throws NoTransporterException + { + + final String instanceKey = JETTY_INSTANCE_KEY_PREFIX + repository.getId(); + + try + { + return (HttpClient) session.getData().computeIfAbsent( instanceKey, () -> + { + SSLContext sslContext = null; + BasicAuthentication basicAuthentication = null; + try + { + try ( AuthenticationContext repoAuthContext = AuthenticationContext.forRepository( session, + repository ) ) + { + if ( repoAuthContext != null ) + { + sslContext = repoAuthContext.get( AuthenticationContext.SSL_CONTEXT, SSLContext.class ); + + String username = repoAuthContext.get( AuthenticationContext.USERNAME ); + String password = repoAuthContext.get( AuthenticationContext.PASSWORD ); + + basicAuthentication = + new BasicAuthentication( URI.create( repository.getUrl() ), + Authentication.ANY_REALM, username, password ); + } + } + + if ( sslContext == null ) + { + sslContext = SSLContext.getDefault(); + } + + int connectTimeout = ConfigUtils.getInteger( session, + ConfigurationProperties.DEFAULT_CONNECT_TIMEOUT, + ConfigurationProperties.CONNECT_TIMEOUT + "." + repository.getId(), + ConfigurationProperties.CONNECT_TIMEOUT ); + + SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(); + sslContextFactory.setSslContext( sslContext ); + + ClientConnector clientConnector = new ClientConnector(); + clientConnector.setSslContextFactory( sslContextFactory ); + + HTTP2Client http2Client = new HTTP2Client( clientConnector ); + ClientConnectionFactoryOverHTTP2.HTTP2 http2 = + new ClientConnectionFactoryOverHTTP2.HTTP2( http2Client ); + + HttpClientTransportDynamic transport; + if ( "https".equalsIgnoreCase( repository.getProtocol() ) ) + { + transport = new HttpClientTransportDynamic( clientConnector, + http2, HttpClientConnectionFactory.HTTP11 ); // HTTPS, prefer H2 + } + else + { + transport = new HttpClientTransportDynamic( clientConnector, + HttpClientConnectionFactory.HTTP11, http2 ); // plaintext HTTP, H2 cannot be used + } + + HttpClient httpClient = new HttpClient( transport ); + httpClient.setConnectTimeout( connectTimeout ); + httpClient.setFollowRedirects( true ); + httpClient.setMaxRedirects( 2 ); + + httpClient.setUserAgentField( null ); // we manage it + + if ( basicAuthentication != null ) + { + httpClient.getAuthenticationStore().addAuthentication( basicAuthentication ); + } + + if ( repository.getProxy() != null ) + { + HttpProxy proxy = + new HttpProxy( repository.getProxy().getHost(), repository.getProxy().getPort() ); + + httpClient.getProxyConfiguration().addProxy( proxy ); + try ( AuthenticationContext proxyAuthContext = AuthenticationContext.forProxy( session, + repository ) ) + { + if ( proxyAuthContext != null ) + { + String username = proxyAuthContext.get( AuthenticationContext.USERNAME ); + String password = proxyAuthContext.get( AuthenticationContext.PASSWORD ); + + BasicAuthentication proxyAuthentication = + new BasicAuthentication( proxy.getURI(), Authentication.ANY_REALM, + username, password ); + + httpClient.getAuthenticationStore().addAuthentication( proxyAuthentication ); + } + } + } + httpClient.start(); Review Comment: is there any call to `stop()` to cleanup? Shouldn't be a problem for `mvn` as jvm is stopped but for `mvnd`? But my understanding of the code there is only one per repo? > HTTP transport showdown > ----------------------- > > Key: MRESOLVER-308 > URL: https://issues.apache.org/jira/browse/MRESOLVER-308 > Project: Maven Resolver > Issue Type: Task > Components: Resolver > Reporter: Tamas Cservenak > Assignee: Tamas Cservenak > Priority: Major > Fix For: 1.9.3 > > > For HTTP protocol resolver currently provides following transports: > * transport-wagon that uses Maven 2.x Wagon, that among other protocols > supports HTTP/1.1 as well > * transport-http that uses directly Apache HttpClient 4.x supporting > HTTP/1.1 but provides enhancements in form of "checksum strategy" (almost all > of remote repositories emit those in headers, sparing one HTTP round-trip) > As we saw, is very easy to outperform these as: > * Maven Central supports HTTP/1.1 but also HTTP/2 > * HTTP/3 is on the way as well > An experiment involving Jetty Client far outperformed both of existing > transports, most probably due HTTP/2 support. > So, clients we should consider: > * Jetty Client > * OkHTTP > * Java 11 HttpClient > Point is, to invest into something that (ideally) transparently supports > HTTP/1.1 and HTTP/2, and more ideal would be if even HTTP/3 would be > transparently supported (Jetty 12 works on that). We could then simply > compare these implementations, count in pros and cons, and decide where we > want to go, -- This message was sent by Atlassian Jira (v8.20.10#820010)