Skip to content

Commit

Permalink
[MRESOLVER-375] Provided and trusted checksum fixes (#307)
Browse files Browse the repository at this point in the history
Several key aspects are broken in provided and trusted checksum feature of resolver:
* trusted checksum post processor: the checksum algorithm is not prefixed (as documented)
* the trusted to provided adapter is defunct without MRM being configured
* the existing ProvidedChecksumsSource interface is broken, needs to be replaced

---

https://issues.apache.org/jira/browse/MRESOLVER-375
  • Loading branch information
cstamas committed Jul 19, 2023
1 parent 0d7cb6e commit 1270cbc
Show file tree
Hide file tree
Showing 9 changed files with 201 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.eclipse.aether.RequestTrace;
import org.eclipse.aether.metadata.Metadata;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.spi.checksums.ProvidedChecksumsSource;
import org.eclipse.aether.spi.connector.ArtifactDownload;
import org.eclipse.aether.spi.connector.ArtifactUpload;
import org.eclipse.aether.spi.connector.MetadataDownload;
Expand All @@ -46,7 +47,6 @@
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmHelper;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider;
import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
import org.eclipse.aether.spi.connector.layout.RepositoryLayoutProvider;
import org.eclipse.aether.spi.connector.transport.GetTask;
Expand Down Expand Up @@ -223,7 +223,7 @@ public void get(
Map<String, String> providedChecksums = Collections.emptyMap();
for (ProvidedChecksumsSource providedChecksumsSource : providedChecksumsSources.values()) {
Map<String, String> provided = providedChecksumsSource.getProvidedArtifactChecksums(
session, transfer, checksumAlgorithmFactories);
session, transfer, repository, checksumAlgorithmFactories);

if (provided != null) {
providedChecksums = provided;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@

import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.spi.checksums.ProvidedChecksumsSource;
import org.eclipse.aether.spi.connector.RepositoryConnector;
import org.eclipse.aether.spi.connector.RepositoryConnectorFactory;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider;
import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
import org.eclipse.aether.spi.connector.layout.RepositoryLayoutProvider;
import org.eclipse.aether.spi.connector.transport.TransporterProvider;
import org.eclipse.aether.spi.io.FileProcessor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@
import org.eclipse.aether.named.providers.LocalReadWriteLockNamedLockFactory;
import org.eclipse.aether.named.providers.LocalSemaphoreNamedLockFactory;
import org.eclipse.aether.named.providers.NoopNamedLockFactory;
import org.eclipse.aether.spi.checksums.ProvidedChecksumsSource;
import org.eclipse.aether.spi.checksums.TrustedChecksumsSource;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactorySelector;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider;
import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
import org.eclipse.aether.spi.connector.filter.RemoteRepositoryFilterSource;
import org.eclipse.aether.spi.connector.layout.RepositoryLayoutFactory;
import org.eclipse.aether.spi.connector.layout.RepositoryLayoutProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,8 @@ protected Map<String, String> doGetTrustedArtifactChecksums(
for (ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories) {
Path summaryFile = summaryFile(
basedir, originAware, artifactRepository.getId(), checksumAlgorithmFactory.getFileExtension());
ConcurrentHashMap<String, String> algorithmChecksums = checksums.computeIfAbsent(summaryFile, f -> {
ConcurrentHashMap<String, String> loaded = loadProvidedChecksums(summaryFile);
if (Files.isRegularFile(summaryFile)) {
LOGGER.info(
"Loaded {} {} trusted checksums for remote repository {}",
loaded.size(),
checksumAlgorithmFactory.getName(),
artifactRepository.getId());
}
return loaded;
});
ConcurrentHashMap<String, String> algorithmChecksums =
checksums.computeIfAbsent(summaryFile, f -> loadProvidedChecksums(summaryFile));
String checksum = algorithmChecksums.get(artifactPath);
if (checksum != null) {
result.put(checksumAlgorithmFactory.getName(), checksum);
Expand Down Expand Up @@ -205,6 +196,7 @@ private ConcurrentHashMap<String, String> loadProvidedChecksums(Path summaryFile
} catch (IOException e) {
throw new UncheckedIOException(e);
}
LOGGER.info("Loaded {} trusted checksums from {}", result.size(), summaryFile);
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.spi.checksums.ProvidedChecksumsSource;
import org.eclipse.aether.spi.checksums.TrustedChecksumsSource;
import org.eclipse.aether.spi.connector.ArtifactDownload;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;

import static java.util.Objects.requireNonNull;

Expand All @@ -57,14 +57,27 @@ public TrustedToProvidedChecksumsSourceAdapter(Map<String, TrustedChecksumsSourc
public Map<String, String> getProvidedArtifactChecksums(
RepositorySystemSession session,
ArtifactDownload transfer,
RemoteRepository repository,
List<ChecksumAlgorithmFactory> checksumAlgorithmFactories) {
Artifact artifact = transfer.getArtifact();
for (RemoteRepository remoteRepository : transfer.getRepositories()) {
for (TrustedChecksumsSource trustedChecksumsSource : trustedChecksumsSources.values()) {
Map<String, String> trustedChecksums = trustedChecksumsSource.getTrustedArtifactChecksums(
session, artifact, remoteRepository, checksumAlgorithmFactories);
if (trustedChecksums != null && !trustedChecksums.isEmpty()) {
return trustedChecksums;
Map<String, String> trustedChecksums;
// check for connector repository
for (TrustedChecksumsSource trustedChecksumsSource : trustedChecksumsSources.values()) {
trustedChecksums = trustedChecksumsSource.getTrustedArtifactChecksums(
session, artifact, repository, checksumAlgorithmFactories);
if (trustedChecksums != null && !trustedChecksums.isEmpty()) {
return trustedChecksums;
}
}
// if repo above is "mirrorOf", this one kicks in
if (!transfer.getRepositories().isEmpty()) {
for (RemoteRepository remoteRepository : transfer.getRepositories()) {
for (TrustedChecksumsSource trustedChecksumsSource : trustedChecksumsSources.values()) {
trustedChecksums = trustedChecksumsSource.getTrustedArtifactChecksums(
session, artifact, remoteRepository, checksumAlgorithmFactories);
if (trustedChecksums != null && !trustedChecksums.isEmpty()) {
return trustedChecksums;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@
import org.eclipse.aether.transfer.ChecksumFailureException;
import org.eclipse.aether.util.ConfigUtils;
import org.eclipse.aether.util.artifact.ArtifactIdUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -92,8 +90,6 @@ public final class TrustedChecksumsArtifactResolverPostProcessor extends Artifac
private static final String CHECKSUM_ALGORITHMS_CACHE_KEY =
TrustedChecksumsArtifactResolverPostProcessor.class.getName() + ".checksumAlgorithms";

private static final Logger LOGGER = LoggerFactory.getLogger(TrustedChecksumsArtifactResolverPostProcessor.class);

private final ChecksumAlgorithmFactorySelector checksumAlgorithmFactorySelector;

private final Map<String, TrustedChecksumsSource> trustedChecksumsSources;
Expand All @@ -115,7 +111,9 @@ protected void doPostProcess(RepositorySystemSession session, List<ArtifactResul
CHECKSUM_ALGORITHMS_CACHE_KEY,
() -> checksumAlgorithmFactorySelector.selectList(
ConfigUtils.parseCommaSeparatedUniqueNames(ConfigUtils.getString(
session, DEFAULT_CHECKSUM_ALGORITHMS, CONF_NAME_CHECKSUM_ALGORITHMS))));
session,
DEFAULT_CHECKSUM_ALGORITHMS,
configPropKey(CONF_NAME_CHECKSUM_ALGORITHMS)))));

final boolean failIfMissing = ConfigUtils.getBoolean(session, false, configPropKey(CONF_NAME_FAIL_IF_MISSING));
final boolean record = ConfigUtils.getBoolean(session, false, configPropKey(CONF_NAME_RECORD));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* 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.eclipse.aether.internal.impl.checksum;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.artifact.DefaultArtifact;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.spi.checksums.TrustedChecksumsSource;
import org.eclipse.aether.spi.connector.ArtifactDownload;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
import org.junit.Before;
import org.junit.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class TrustedToProvidedChecksumsSourceAdapterTest {
private final Artifact artifactWithChecksum = new DefaultArtifact("g:a:v1");

private final Artifact artifactWithoutChecksum = new DefaultArtifact("g:a:v2");

private final RemoteRepository repository =
new RemoteRepository.Builder("repo", "default", "https://example.com").build();

private final List<ChecksumAlgorithmFactory> checksums =
Collections.singletonList(new Sha1ChecksumAlgorithmFactory());

private final RepositorySystemSession session = mock(RepositorySystemSession.class);

private final TrustedChecksumsSource trustedChecksumsSource = mock(TrustedChecksumsSource.class);

private TrustedToProvidedChecksumsSourceAdapter adapter;

@Before
public void before() {
HashMap<String, String> result = new HashMap<>();
result.put(Sha1ChecksumAlgorithmFactory.NAME, "foo");
when(trustedChecksumsSource.getTrustedArtifactChecksums(
eq(session), eq(artifactWithChecksum), eq(repository), eq(checksums)))
.thenReturn(result);
adapter = new TrustedToProvidedChecksumsSourceAdapter(
Collections.singletonMap("trusted", trustedChecksumsSource));
}

@Test
public void testSimplePositive() {
ArtifactDownload transfer = new ArtifactDownload();
transfer.setArtifact(artifactWithChecksum);
Map<String, String> chk = adapter.getProvidedArtifactChecksums(session, transfer, repository, checksums);
assertThat(chk, notNullValue());
assertThat(chk, hasEntry(Sha1ChecksumAlgorithmFactory.NAME, "foo"));
}

@Test
public void testSimpleNegative() {
ArtifactDownload transfer = new ArtifactDownload();
transfer.setArtifact(artifactWithoutChecksum);
Map<String, String> chk = adapter.getProvidedArtifactChecksums(session, transfer, repository, checksums);
assertThat(chk, nullValue());
}

@Test
public void testMrmPositive() {
RemoteRepository mrm = new RemoteRepository.Builder("mrm", "default", "https://example.com").build();
ArtifactDownload transfer = new ArtifactDownload();
transfer.setArtifact(artifactWithChecksum);
transfer.setRepositories(Collections.singletonList(repository));
Map<String, String> chk = adapter.getProvidedArtifactChecksums(session, transfer, mrm, checksums);
assertThat(chk, notNullValue());
assertThat(chk, hasEntry(Sha1ChecksumAlgorithmFactory.NAME, "foo"));
}

@Test
public void testMrmNegative() {
RemoteRepository mrm = new RemoteRepository.Builder("mrm", "default", "https://example.com").build();
ArtifactDownload transfer = new ArtifactDownload();
transfer.setArtifact(artifactWithoutChecksum);
transfer.setRepositories(Collections.singletonList(repository));
Map<String, String> chk = adapter.getProvidedArtifactChecksums(session, transfer, mrm, checksums);
assertThat(chk, nullValue());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* 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.eclipse.aether.spi.checksums;

import java.util.List;
import java.util.Map;

import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.spi.connector.ArtifactDownload;
import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy;

/**
* Component able to provide (expected) checksums to connector beforehand the download happens. Checksum provided by
* this component are of kind {@link ChecksumPolicy.ChecksumKind#PROVIDED}. Resolver by default provides one
* implementation: an adapter, that makes {@link TrustedChecksumsSource} into {@link ProvidedChecksumsSource}. Users
* are encouraged to rely on this adapter, and do not create their own implementations.
*
* @since TBD
*/
public interface ProvidedChecksumsSource {
/**
* May return the provided checksums (for given artifact transfer) from source other than remote repository, or
* {@code null} if it have no checksums available for given transfer. Provided checksums are "opt-in" for
* transfer, in a way IF they are available upfront, they will be enforced according to checksum policy
* in effect. Otherwise, provided checksum verification is completely left out.
* <p>
* For enabled provided checksum source is completely acceptable to return {@code null} values, as that carries
* the meaning "nothing to add here", as there are no checksums to be provided upfront transfer. Semantically, this
* is equivalent to returning empty map, but signals the intent better.
*
* @param session The current session.
* @param transfer The transfer that is about to be executed.
* @param remoteRepository The remote repository connector is about to contact.
* @param checksumAlgorithmFactories The checksum algorithms that are expected.
* @return Map of expected checksums, or {@code null}.
*/
Map<String, String> getProvidedArtifactChecksums(
RepositorySystemSession session,
ArtifactDownload transfer,
RemoteRepository remoteRepository,
List<ChecksumAlgorithmFactory> checksumAlgorithmFactories);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
* this component are of kind {@link org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind#PROVIDED}.
*
* @since 1.8.0
* @deprecated This interface is not used anymore, use {@link org.eclipse.aether.spi.checksums.ProvidedChecksumsSource}.
*/
@Deprecated
public interface ProvidedChecksumsSource {
/**
* May return the provided checksums (for given artifact transfer) from source other than remote repository, or
Expand Down

0 comments on commit 1270cbc

Please sign in to comment.