Honour configured policy and validate inputs on the legacy compat paths

maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java

@@ -35,6 +35,7 @@
 import org.apache.maven.artifact.repository.RepositoryRequest;
 import org.apache.maven.artifact.repository.metadata.io.xpp3.MetadataXpp3Writer;
 import org.apache.maven.repository.internal.metadata.ValidatingMetadataXpp3Reader;
+import org.apache.maven.repository.legacy.ChecksumFailedException;
 import org.apache.maven.repository.legacy.UpdateCheckManager;
 import org.apache.maven.repository.legacy.WagonManager;
 import org.apache.maven.wagon.ResourceDoesNotExistException;
@@ -114,6 +115,17 @@ public void resolve(RepositoryMetadata metadata, RepositoryRequest request)
                     getLogger().info(metadata.getKey() + ": checking for updates from " + repository.getId());
                     try {
                         wagonManager.getArtifactMetadata(metadata, repository, file, policy.getChecksumPolicy());
+                        updateCheckManager.touch(metadata, repository, file);
+                    } catch (ChecksumFailedException e) {
+                        // ChecksumFailedException is only thrown by the wagon manager under
+                        // CHECKSUM_POLICY_FAIL: honor the strict policy by failing metadata resolution
+                        // instead of downgrading the integrity failure to a warning. The update
+                        // tracking file is deliberately not touched, so the next build retries
+                        // immediately instead of trusting stale metadata for a full update interval.
+                        throw new RepositoryMetadataResolutionException(
+                                metadata + " failed checksum verification against repository: " + repository.getId()
+                                        + " due to an error: " + e.getMessage(),
+                                e);
                     } catch (ResourceDoesNotExistException e) {
                         getLogger().debug(metadata + " could not be found on repository: " + repository.getId());
 
@@ -129,12 +141,12 @@ public void resolve(RepositoryMetadata metadata, RepositoryRequest request)
                                 file.delete(); // if this fails, forget about it
                             }
                         }
+                        updateCheckManager.touch(metadata, repository, file);
                     } catch (TransferFailedException e) {
                         getLogger()
                                 .warn(metadata + " could not be retrieved from repository: " + repository.getId()
                                         + " due to an error: " + e.getMessage());
                         getLogger().debug("Exception", e);
-                    } finally {
                         updateCheckManager.touch(metadata, repository, file);
                     }
                 }
@@ -281,6 +293,9 @@ protected Metadata readMetadata(File mappingFile) throws RepositoryMetadataReadE
             throw new RepositoryMetadataReadException(
                     "Cannot read metadata from '" + mappingFile + "': " + e.getMessage(), e);
         }
+
+        validateVersioning(result);
+
         return result;
     }
 
@@ -401,7 +416,7 @@ private File getArtifactMetadataFromDeploymentRepository(
 
         try {
             wagonManager.getArtifactMetadataFromDeploymentRepository(
-                    metadata, remoteRepository, file, ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN);
+                    metadata, remoteRepository, file, getChecksumPolicy(metadata, remoteRepository));
         } catch (ResourceDoesNotExistException e) {
             getLogger()
                     .info(metadata + " could not be found on repository: " + remoteRepository.getId()
@@ -427,6 +442,22 @@ private File getArtifactMetadataFromDeploymentRepository(
         return file;
     }
 
+    /**
+     * Determines the effective checksum policy for a transfer from the given repository. The
+     * operator-configured policy (e.g. {@code fail} via {@code -C}/{@code --strict-checksums} or a
+     * per-repository {@code checksumPolicy}) must govern every remote transfer, so it must not be
+     * hardcoded at the call sites.
+     */
+    private String getChecksumPolicy(ArtifactMetadata metadata, ArtifactRepository repository) {
+        if (metadata instanceof RepositoryMetadata) {
+            ArtifactRepositoryPolicy policy = ((RepositoryMetadata) metadata).getPolicy(repository);
+            if (policy != null && policy.getChecksumPolicy() != null) {
+                return policy.getChecksumPolicy();
+            }
+        }
+        return ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN;
+    }
+
     public void deploy(
             ArtifactMetadata metadata, ArtifactRepository localRepository, ArtifactRepository deploymentRepository)
             throws RepositoryMetadataDeploymentException {

maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java

@@ -662,7 +662,7 @@ public void retrieve(
                     destination,
                     remotePath,
                     TransferListenerAdapter.newAdapter(transferListener),
-                    ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN,
+                    getChecksumPolicy(repository),
                     true);
         } catch (org.apache.maven.wagon.TransferFailedException e) {
             throw new ArtifactTransferFailedException(getMessage(e, "Error transferring artifact."), e);
@@ -671,6 +671,39 @@ public void retrieve(
         }
     }
 
+    /**
+     * Determines the effective checksum policy for a generic retrieval from the given repository.
+     * The operator-configured policy (e.g. {@code fail} via {@code -C}/{@code --strict-checksums})
+     * must govern every remote transfer instead of a hardcoded lenient default. A generic remote
+     * path cannot be classified as release or snapshot, so the stricter of the two configured
+     * policies applies.
+     */
+    private static String getChecksumPolicy(ArtifactRepository repository) {
+        String releases =
+                (repository.getReleases() != null) ? repository.getReleases().getChecksumPolicy() : null;
+        String snapshots =
+                (repository.getSnapshots() != null) ? repository.getSnapshots().getChecksumPolicy() : null;
+        String policy;
+        if (releases == null) {
+            policy = snapshots;
+        } else if (snapshots == null || checksumRank(releases) >= checksumRank(snapshots)) {
+            policy = releases;
+        } else {
+            policy = snapshots;
+        }
+        return (policy != null) ? policy : ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN;
+    }
+
+    private static int checksumRank(String policy) {
+        if (ArtifactRepositoryPolicy.CHECKSUM_POLICY_FAIL.equals(policy)) {
+            return 2;
+        } else if (ArtifactRepositoryPolicy.CHECKSUM_POLICY_IGNORE.equals(policy)) {
+            return 0;
+        } else {
+            return 1;
+        }
+    }
+
     public void publish(
             ArtifactRepository repository, File source, String remotePath, ArtifactTransferListener transferListener)
             throws ArtifactTransferFailedException {

maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java

@@ -18,23 +18,74 @@
  */
 package org.apache.maven.artifact.repository.metadata;
 
+import javax.inject.Inject;
+import javax.inject.Named;
+
 import java.io.File;
 import java.net.URL;
+import java.util.Collections;
 
+import org.apache.maven.artifact.AbstractArtifactComponentTest;
+import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
+import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
+import org.codehaus.plexus.testing.PlexusTest;
+import org.codehaus.plexus.util.FileUtils;
 import org.junit.jupiter.api.Test;
 
+import static org.codehaus.plexus.testing.PlexusExtension.getBasedir;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
- * Tests that {@link DefaultRepositoryMetadataManager} rejects repository metadata carrying version tokens that
- * are not valid coordinate components, on the legacy read path used when metadata is loaded for merging.
+ * Tests {@link DefaultRepositoryMetadataManager}.
  */
-public class DefaultRepositoryMetadataManagerTest {
+@PlexusTest
+@Deprecated
+class DefaultRepositoryMetadataManagerTest extends AbstractArtifactComponentTest {
+
+    @Inject
+    private RepositoryMetadataManager repositoryMetadataManager;
+
+    @Inject
+    @Named("default")
+    private ArtifactRepositoryLayout layout;
 
     private final DefaultRepositoryMetadataManager manager = new DefaultRepositoryMetadataManager();
 
+    @Override
+    protected String component() {
+        return "repositoryMetadataManager";
+    }
+
+    @Test
+    void testResolveHonorsConfiguredFailChecksumPolicy() throws Exception {
+        RepositoryMetadata metadata = new GroupRepositoryMetadata("checksum-policy-test-group");
+
+        ArtifactRepositoryPolicy failPolicy = new ArtifactRepositoryPolicy(
+                true, ArtifactRepositoryPolicy.UPDATE_POLICY_ALWAYS, ArtifactRepositoryPolicy.CHECKSUM_POLICY_FAIL);
+
+        File remoteBase = new File(getBasedir(), "target/test-repositories/" + component() + "/remote-repository");
+        FileUtils.deleteDirectory(remoteBase);
+
+        ArtifactRepository remoteRepo = artifactRepositoryFactory.createArtifactRepository(
+                "test", "file://" + remoteBase.getPath(), layout, failPolicy, failPolicy);
+
+        String remotePath = remoteRepo.pathOfRemoteRepositoryMetadata(metadata);
+        File remoteFile = new File(remoteBase, remotePath);
+        remoteFile.getParentFile().mkdirs();
+        FileUtils.fileWrite(remoteFile.getAbsolutePath(), "<metadata/>");
+        FileUtils.fileWrite(remoteFile.getAbsolutePath() + ".sha1", "0000000000000000000000000000000000000000");
+
+        ArtifactRepository localRepo = localRepository();
+        FileUtils.deleteDirectory(new File(localRepo.getBasedir()));
+
+        assertThrows(
+                RepositoryMetadataResolutionException.class,
+                () -> repositoryMetadataManager.resolve(metadata, Collections.singletonList(remoteRepo), localRepo));
+    }
+
     @Test
     void testMetadataWithInvalidVersionTokenIsRejected() {
         File metadataFile = testFile("metadata-invalid-token/maven-metadata.xml");

maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java

@@ -0,0 +1,63 @@
+/*
+ * 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.apache.maven.artifact.repository.metadata;
+
+import java.io.File;
+import java.net.URL;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests that {@link DefaultRepositoryMetadataManager} rejects repository metadata carrying version tokens that
+ * are not valid coordinate components, on the legacy read path used when metadata is loaded for merging.
+ */
+class DefaultRepositoryMetadataManagerValidationTest {
+
+    private final DefaultRepositoryMetadataManager manager = new DefaultRepositoryMetadataManager();
+
+    @Test
+    void testMetadataWithInvalidVersionTokenIsRejected() {
+        File metadataFile = testFile("metadata-invalid-token/maven-metadata.xml");
+
+        RepositoryMetadataReadException exception =
+                assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile));
+
+        assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage());
+    }
+
+    @Test
+    void testMetadataWithInvalidSnapshotTimestampIsRejected() {
+        File metadataFile = testFile("metadata-invalid-timestamp/maven-metadata.xml");
+
+        RepositoryMetadataReadException exception =
+                assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile));
+
+        assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage());
+    }
+
+    private static File testFile(String resource) {
+        URL url = Thread.currentThread().getContextClassLoader().getResource(resource);
+        assertNotNull(url, "test resource not found: " + resource);
+        return new File(url.getFile());
+    }
+}

maven-compat/src/test/java/org/apache/maven/repository/legacy/LegacyRepositorySystemTest.java

@@ -21,17 +21,22 @@
 import javax.inject.Inject;
 
 import java.io.File;
+import java.nio.file.Files;
 import java.util.Arrays;
 
 import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
 import org.apache.maven.artifact.repository.Authentication;
+import org.apache.maven.repository.ArtifactTransferFailedException;
 import org.apache.maven.repository.RepositorySystem;
 import org.apache.maven.settings.Server;
 import org.codehaus.plexus.testing.PlexusTest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Tests {@link LegacyRepositorySystem}.
@@ -66,4 +71,26 @@ public void testAuthenticationHandling() throws Exception {
         assertEquals("jason", authentication.getUsername());
         assertEquals("abc123", authentication.getPassword());
     }
+
+    @Test
+    void testRetrieveHonorsConfiguredFailChecksumPolicy(@TempDir File tempDir) throws Exception {
+        File remoteBase = new File(tempDir, "remote");
+        remoteBase.mkdirs();
+        File remoteFile = new File(remoteBase, "sample.txt");
+        Files.write(remoteFile.toPath(), "content".getBytes());
+        Files.write(
+                new File(remoteBase, "sample.txt.sha1").toPath(),
+                "0000000000000000000000000000000000000000".getBytes());
+
+        ArtifactRepositoryPolicy failPolicy = new ArtifactRepositoryPolicy(
+                true, ArtifactRepositoryPolicy.UPDATE_POLICY_ALWAYS, ArtifactRepositoryPolicy.CHECKSUM_POLICY_FAIL);
+        ArtifactRepository repository = repositorySystem.createArtifactRepository(
+                "test", "file://" + remoteBase.getAbsolutePath(), null, failPolicy, failPolicy);
+
+        File destination = new File(tempDir, "sample.txt");
+
+        assertThrows(
+                ArtifactTransferFailedException.class,
+                () -> repositorySystem.retrieve(repository, destination, "sample.txt", null));
+    }
 }

maven-compat/src/test/resources/metadata-invalid-timestamp/maven-metadata.xml

@@ -1,30 +1,26 @@
 <?xml version="1.0" encoding="UTF-8"?>
-
 <!--
-  ~ 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.
--->
+  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
 
-<metadata xmlns="http://maven.apache.org/METADATA/1.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-  xsi:schemaLocation="http://maven.apache.org/METADATA/1.1.0 http://maven.apache.org/xsd/metadata-1.1.0.xsd"
-  modelVersion="1.1.0">
+    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.
+-->
+<metadata>
   <groupId>org.apache.maven.its</groupId>
   <artifactId>dep-invalid-timestamp</artifactId>
-  <version>1.0-SNAPSHOT</version><!-- metadata with an invalid snapshot timestamp token -->
+  <version>1.0-SNAPSHOT</version>
   <versioning>
     <snapshot>
       <timestamp>20120809.112920:1</timestamp>