[
https://issues.apache.org/jira/browse/GROOVY-12265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105845#comment-18105845
]
ASF GitHub Bot commented on GROOVY-12265:
-----------------------------------------
Copilot commented on code in PR #2802:
URL: https://github.com/apache/groovy/pull/2802#discussion_r3811485523
##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeChecksumPolicyProvider.groovy:
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 groovy.grape.maven
+
+import groovy.transform.AutoFinal
+import groovy.transform.CompileStatic
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider
Review Comment:
The production implementation depends directly on
`org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider` (an internal
class), which is more likely to break across Maven Resolver upgrades. A more
robust approach is to inject the delegate `ChecksumPolicyProvider` (e.g., from
`RepositorySystemSupplier`/`super.createChecksumPolicyProvider()`), so this
class only depends on the public `ChecksumPolicyProvider` SPI.
##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeChecksumPolicyProvider.groovy:
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 groovy.grape.maven
+
+import groovy.transform.AutoFinal
+import groovy.transform.CompileStatic
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider
+import org.eclipse.aether.transfer.ChecksumFailureException
+import org.eclipse.aether.transfer.TransferResource
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+/**
+ * Supplies the checksum policy Grape uses for remote artifact downloads.
+ *
+ * <p>Maven Resolver's stock {@code CHECKSUM_POLICY_FAIL} rejects a download
both when a
+ * published checksum does not match and when no checksum is published at all.
Grape wants
+ * only the first of those: a mismatch means the bytes are not what the
repository said they
+ * would be and must never reach the classpath, whereas a repository that
simply publishes no
+ * {@code .sha1}/{@code .md5} is a common and legitimate situation,
particularly for internal
+ * and older repositories.
+ *
+ * <p>This provider therefore delegates to {@link
DefaultChecksumPolicyProvider} for every
+ * policy and wraps only the {@code fail} policy, relaxing its
+ * {@link ChecksumPolicy#onNoMoreChecksums()} response. The result matches the
semantics the
+ * Ivy-backed engine has always had, so both Grape engines behave alike.
+ *
+ * @since 6.0.0
+ */
+@AutoFinal
+@CompileStatic
+class GrapeChecksumPolicyProvider implements ChecksumPolicyProvider {
+
+ private final ChecksumPolicyProvider delegate = new
DefaultChecksumPolicyProvider()
+
+ /**
+ * Returns the policy for the given resource, relaxing {@code fail} to
tolerate artifacts
+ * that publish no checksum.
+ *
+ * @param session the session during which the request is made
+ * @param repository the repository hosting the resource
+ * @param resource the resource the policy will be applied to
+ * @param policy the identifier of the policy to apply
+ * @return the policy to apply, or {@code null} if checksums should be
ignored
+ */
+ @Override
+ ChecksumPolicy newChecksumPolicy(RepositorySystemSession session,
RemoteRepository repository, TransferResource resource, String policy) {
+ ChecksumPolicy checksumPolicy = delegate.newChecksumPolicy(session,
repository, resource, policy)
+ if (checksumPolicy != null && RepositoryPolicy.CHECKSUM_POLICY_FAIL ==
policy) {
+ return new AbsenceTolerantChecksumPolicy(checksumPolicy, resource)
+ }
+ checksumPolicy
+ }
+
+ /**
+ * Returns the least strict of the two supplied policies.
+ *
+ * @param session the session during which the request is made
+ * @param policy1 a policy to compare
+ * @param policy2 a policy to compare
+ * @return the least strict policy of the two
+ */
+ @Override
+ String getEffectiveChecksumPolicy(RepositorySystemSession session, String
policy1, String policy2) {
+ delegate.getEffectiveChecksumPolicy(session, policy1, policy2)
+ }
+
+ /**
+ * Wraps a checksum policy so that the absence of any published checksum
is tolerated while
+ * every other outcome, in particular a mismatch, is left to the wrapped
policy.
+ */
+ @AutoFinal
+ @CompileStatic
+ private static class AbsenceTolerantChecksumPolicy implements
ChecksumPolicy {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(AbsenceTolerantChecksumPolicy)
+
+ private final ChecksumPolicy delegate
+ private final TransferResource resource
+
+ AbsenceTolerantChecksumPolicy(ChecksumPolicy delegate,
TransferResource resource) {
+ this.delegate = delegate
+ this.resource = resource
+ }
+
+ @Override
+ boolean onChecksumMatch(String algorithm, ChecksumKind kind) {
+ delegate.onChecksumMatch(algorithm, kind)
+ }
+
+ @Override
+ void onChecksumMismatch(String algorithm, ChecksumKind kind,
ChecksumFailureException exception) throws ChecksumFailureException {
+ delegate.onChecksumMismatch(algorithm, kind, exception)
+ }
+
+ @Override
+ void onChecksumError(String algorithm, ChecksumKind kind,
ChecksumFailureException exception) throws ChecksumFailureException {
+ delegate.onChecksumError(algorithm, kind, exception)
+ }
Review Comment:
As implemented, `onNoMoreChecksums()` will also accept artifacts when
checksum retrieval failed (i.e., `onChecksumError` was invoked but no checksum
was ultimately validated). That weakens integrity guarantees beyond “repository
publishes no checksums”. Consider tracking whether `onChecksumError(...)`
occurred in this wrapper and, if so, delegate to the underlying fail behavior
(or rethrow) in `onNoMoreChecksums()`, while only tolerating the true ‘no
checksums published’ path.
##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeChecksumPolicyProvider.groovy:
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 groovy.grape.maven
+
+import groovy.transform.AutoFinal
+import groovy.transform.CompileStatic
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider
+import org.eclipse.aether.transfer.ChecksumFailureException
+import org.eclipse.aether.transfer.TransferResource
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+/**
+ * Supplies the checksum policy Grape uses for remote artifact downloads.
+ *
+ * <p>Maven Resolver's stock {@code CHECKSUM_POLICY_FAIL} rejects a download
both when a
+ * published checksum does not match and when no checksum is published at all.
Grape wants
+ * only the first of those: a mismatch means the bytes are not what the
repository said they
+ * would be and must never reach the classpath, whereas a repository that
simply publishes no
+ * {@code .sha1}/{@code .md5} is a common and legitimate situation,
particularly for internal
+ * and older repositories.
+ *
+ * <p>This provider therefore delegates to {@link
DefaultChecksumPolicyProvider} for every
+ * policy and wraps only the {@code fail} policy, relaxing its
+ * {@link ChecksumPolicy#onNoMoreChecksums()} response. The result matches the
semantics the
+ * Ivy-backed engine has always had, so both Grape engines behave alike.
+ *
+ * @since 6.0.0
+ */
+@AutoFinal
+@CompileStatic
+class GrapeChecksumPolicyProvider implements ChecksumPolicyProvider {
+
+ private final ChecksumPolicyProvider delegate = new
DefaultChecksumPolicyProvider()
Review Comment:
The production implementation depends directly on
`org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider` (an internal
class), which is more likely to break across Maven Resolver upgrades. A more
robust approach is to inject the delegate `ChecksumPolicyProvider` (e.g., from
`RepositorySystemSupplier`/`super.createChecksumPolicyProvider()`), so this
class only depends on the public `ChecksumPolicyProvider` SPI.
##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeChecksumPolicyProvider.groovy:
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 groovy.grape.maven
+
+import groovy.transform.AutoFinal
+import groovy.transform.CompileStatic
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider
+import org.eclipse.aether.transfer.ChecksumFailureException
+import org.eclipse.aether.transfer.TransferResource
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+/**
+ * Supplies the checksum policy Grape uses for remote artifact downloads.
+ *
+ * <p>Maven Resolver's stock {@code CHECKSUM_POLICY_FAIL} rejects a download
both when a
+ * published checksum does not match and when no checksum is published at all.
Grape wants
+ * only the first of those: a mismatch means the bytes are not what the
repository said they
+ * would be and must never reach the classpath, whereas a repository that
simply publishes no
+ * {@code .sha1}/{@code .md5} is a common and legitimate situation,
particularly for internal
+ * and older repositories.
+ *
+ * <p>This provider therefore delegates to {@link
DefaultChecksumPolicyProvider} for every
+ * policy and wraps only the {@code fail} policy, relaxing its
+ * {@link ChecksumPolicy#onNoMoreChecksums()} response. The result matches the
semantics the
+ * Ivy-backed engine has always had, so both Grape engines behave alike.
+ *
+ * @since 6.0.0
+ */
+@AutoFinal
+@CompileStatic
+class GrapeChecksumPolicyProvider implements ChecksumPolicyProvider {
+
+ private final ChecksumPolicyProvider delegate = new
DefaultChecksumPolicyProvider()
+
+ /**
+ * Returns the policy for the given resource, relaxing {@code fail} to
tolerate artifacts
+ * that publish no checksum.
+ *
+ * @param session the session during which the request is made
+ * @param repository the repository hosting the resource
+ * @param resource the resource the policy will be applied to
+ * @param policy the identifier of the policy to apply
+ * @return the policy to apply, or {@code null} if checksums should be
ignored
+ */
+ @Override
+ ChecksumPolicy newChecksumPolicy(RepositorySystemSession session,
RemoteRepository repository, TransferResource resource, String policy) {
+ ChecksumPolicy checksumPolicy = delegate.newChecksumPolicy(session,
repository, resource, policy)
+ if (checksumPolicy != null && RepositoryPolicy.CHECKSUM_POLICY_FAIL ==
policy) {
+ return new AbsenceTolerantChecksumPolicy(checksumPolicy, resource)
+ }
+ checksumPolicy
+ }
+
+ /**
+ * Returns the least strict of the two supplied policies.
+ *
+ * @param session the session during which the request is made
+ * @param policy1 a policy to compare
+ * @param policy2 a policy to compare
+ * @return the least strict policy of the two
+ */
+ @Override
+ String getEffectiveChecksumPolicy(RepositorySystemSession session, String
policy1, String policy2) {
+ delegate.getEffectiveChecksumPolicy(session, policy1, policy2)
+ }
+
+ /**
+ * Wraps a checksum policy so that the absence of any published checksum
is tolerated while
+ * every other outcome, in particular a mismatch, is left to the wrapped
policy.
+ */
+ @AutoFinal
+ @CompileStatic
+ private static class AbsenceTolerantChecksumPolicy implements
ChecksumPolicy {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(AbsenceTolerantChecksumPolicy)
+
+ private final ChecksumPolicy delegate
+ private final TransferResource resource
+
+ AbsenceTolerantChecksumPolicy(ChecksumPolicy delegate,
TransferResource resource) {
+ this.delegate = delegate
+ this.resource = resource
+ }
+
+ @Override
+ boolean onChecksumMatch(String algorithm, ChecksumKind kind) {
+ delegate.onChecksumMatch(algorithm, kind)
+ }
+
+ @Override
+ void onChecksumMismatch(String algorithm, ChecksumKind kind,
ChecksumFailureException exception) throws ChecksumFailureException {
+ delegate.onChecksumMismatch(algorithm, kind, exception)
+ }
+
+ @Override
+ void onChecksumError(String algorithm, ChecksumKind kind,
ChecksumFailureException exception) throws ChecksumFailureException {
+ delegate.onChecksumError(algorithm, kind, exception)
+ }
+
+ /**
+ * Accepts the download when no checksum could be validated.
+ *
+ * <p>This is the single point where the policy departs from the
wrapped {@code fail}
+ * policy. It covers both a repository that publishes no checksum and
one whose checksum
+ * could not be retrieved, since Maven Resolver reports a failed
retrieval through
+ * {@link #onChecksumError} and then arrives here having validated
nothing.
+ */
+ @Override
+ void onNoMoreChecksums() {
+ LOG.debug('No checksum could be validated for {}{}; accepting the
artifact unverified',
Review Comment:
The log message concatenates `repositoryUrl` and `resourceName` with `{}{}),
which can produce ambiguous output when the URL does not end with `/`. Consider
logging them with a clear separator (e.g., `'{}/{}'`) or as separate labeled
fields to make troubleshooting easier.
##########
subprojects/groovy-grape-maven/src/test/groovy/groovy/grape/maven/GrapeChecksumPolicyProviderTest.groovy:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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 groovy.grape.maven
+
+import org.eclipse.aether.DefaultRepositorySystemSession
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind
+import org.eclipse.aether.transfer.ChecksumFailureException
+import org.eclipse.aether.transfer.TransferResource
+import org.junit.jupiter.api.Test
+
+import java.util.function.Function
+
+import static groovy.test.GroovyAssert.shouldFail
+
+/**
+ * Tests that Grape's checksum policy rejects mismatched artifacts while
tolerating artifacts
+ * for which no checksum is published.
+ */
+final class GrapeChecksumPolicyProviderTest {
+
+ private static final GrapeChecksumPolicyProvider PROVIDER = new
GrapeChecksumPolicyProvider()
+
+ private static final RepositorySystemSession SESSION =
+ new DefaultRepositorySystemSession({ Runnable r -> Boolean.FALSE } as
Function)
+
+ private static final RemoteRepository REPOSITORY =
+ new RemoteRepository.Builder('test', 'default',
'https://repo.example.invalid/maven2').build()
+
+ private static final TransferResource RESOURCE = new TransferResource(
+ 'test', 'https://repo.example.invalid/maven2',
'org/example/demo/1.0/demo-1.0.jar', null, null, null)
+
+ private static ChecksumPolicy policyFor(String policy) {
+ PROVIDER.newChecksumPolicy(SESSION, REPOSITORY, RESOURCE, policy)
+ }
+
+ private static ChecksumFailureException mismatch() {
+ ChecksumFailureException.mismatch('expected',
ChecksumKind.REMOTE_EXTERNAL.name(), 'actual')
+ }
+
+ @Test
+ void testAbsentChecksumIsToleratedUnderFail() {
+ ChecksumPolicy policy =
policyFor(RepositoryPolicy.CHECKSUM_POLICY_FAIL)
+ // Grape accepts the artifact, matching the Ivy-backed engine.
+ policy.onNoMoreChecksums()
+
+ // Guard the premise: the stock policy this one wraps rejects the same
situation, so
+ // the test above is meaningful and will start failing if Maven
Resolver ever relaxes
+ // CHECKSUM_POLICY_FAIL itself.
+ ChecksumPolicy stock = new DefaultChecksumPolicyProvider()
+ .newChecksumPolicy(SESSION, REPOSITORY, RESOURCE,
RepositoryPolicy.CHECKSUM_POLICY_FAIL)
+ shouldFail(ChecksumFailureException) {
+ stock.onNoMoreChecksums()
+ }
+ }
+
+ @Test
+ void testMismatchedChecksumStillFailsUnderFail() {
+ ChecksumPolicy policy =
policyFor(RepositoryPolicy.CHECKSUM_POLICY_FAIL)
+ ChecksumFailureException expected = mismatch()
+
+ def actual = null
+ try {
+ policy.onChecksumMismatch('SHA-1', ChecksumKind.REMOTE_EXTERNAL,
expected)
+ } catch (ChecksumFailureException e) {
+ actual = e
+ }
+ assert actual.is(expected)
Review Comment:
This test asserts object identity (`is`) for the thrown exception. The
`ChecksumPolicy` contract generally only requires throwing a
`ChecksumFailureException`, not necessarily rethrowing the same instance, so
this can become unnecessarily brittle across resolver implementations/versions.
Prefer asserting that a `ChecksumFailureException` is thrown and (optionally)
that it represents a mismatch via message/cause fields, rather than requiring
the identical instance.
##########
subprojects/groovy-grape-maven/src/main/groovy/groovy/grape/maven/GrapeChecksumPolicyProvider.groovy:
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 groovy.grape.maven
+
+import groovy.transform.AutoFinal
+import groovy.transform.CompileStatic
+import org.eclipse.aether.RepositorySystemSession
+import org.eclipse.aether.internal.impl.DefaultChecksumPolicyProvider
+import org.eclipse.aether.repository.RemoteRepository
+import org.eclipse.aether.repository.RepositoryPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind
+import org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider
+import org.eclipse.aether.transfer.ChecksumFailureException
+import org.eclipse.aether.transfer.TransferResource
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+/**
+ * Supplies the checksum policy Grape uses for remote artifact downloads.
+ *
+ * <p>Maven Resolver's stock {@code CHECKSUM_POLICY_FAIL} rejects a download
both when a
+ * published checksum does not match and when no checksum is published at all.
Grape wants
+ * only the first of those: a mismatch means the bytes are not what the
repository said they
+ * would be and must never reach the classpath, whereas a repository that
simply publishes no
+ * {@code .sha1}/{@code .md5} is a common and legitimate situation,
particularly for internal
+ * and older repositories.
+ *
+ * <p>This provider therefore delegates to {@link
DefaultChecksumPolicyProvider} for every
+ * policy and wraps only the {@code fail} policy, relaxing its
+ * {@link ChecksumPolicy#onNoMoreChecksums()} response. The result matches the
semantics the
+ * Ivy-backed engine has always had, so both Grape engines behave alike.
+ *
+ * @since 6.0.0
+ */
+@AutoFinal
+@CompileStatic
+class GrapeChecksumPolicyProvider implements ChecksumPolicyProvider {
+
+ private final ChecksumPolicyProvider delegate = new
DefaultChecksumPolicyProvider()
+
+ /**
+ * Returns the policy for the given resource, relaxing {@code fail} to
tolerate artifacts
+ * that publish no checksum.
+ *
+ * @param session the session during which the request is made
+ * @param repository the repository hosting the resource
+ * @param resource the resource the policy will be applied to
+ * @param policy the identifier of the policy to apply
+ * @return the policy to apply, or {@code null} if checksums should be
ignored
+ */
+ @Override
+ ChecksumPolicy newChecksumPolicy(RepositorySystemSession session,
RemoteRepository repository, TransferResource resource, String policy) {
+ ChecksumPolicy checksumPolicy = delegate.newChecksumPolicy(session,
repository, resource, policy)
+ if (checksumPolicy != null && RepositoryPolicy.CHECKSUM_POLICY_FAIL ==
policy) {
+ return new AbsenceTolerantChecksumPolicy(checksumPolicy, resource)
+ }
+ checksumPolicy
+ }
+
+ /**
+ * Returns the least strict of the two supplied policies.
+ *
+ * @param session the session during which the request is made
+ * @param policy1 a policy to compare
+ * @param policy2 a policy to compare
+ * @return the least strict policy of the two
+ */
+ @Override
+ String getEffectiveChecksumPolicy(RepositorySystemSession session, String
policy1, String policy2) {
+ delegate.getEffectiveChecksumPolicy(session, policy1, policy2)
+ }
+
+ /**
+ * Wraps a checksum policy so that the absence of any published checksum
is tolerated while
+ * every other outcome, in particular a mismatch, is left to the wrapped
policy.
+ */
+ @AutoFinal
+ @CompileStatic
+ private static class AbsenceTolerantChecksumPolicy implements
ChecksumPolicy {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(AbsenceTolerantChecksumPolicy)
+
+ private final ChecksumPolicy delegate
+ private final TransferResource resource
+
+ AbsenceTolerantChecksumPolicy(ChecksumPolicy delegate,
TransferResource resource) {
+ this.delegate = delegate
+ this.resource = resource
+ }
+
+ @Override
+ boolean onChecksumMatch(String algorithm, ChecksumKind kind) {
+ delegate.onChecksumMatch(algorithm, kind)
+ }
+
+ @Override
+ void onChecksumMismatch(String algorithm, ChecksumKind kind,
ChecksumFailureException exception) throws ChecksumFailureException {
+ delegate.onChecksumMismatch(algorithm, kind, exception)
+ }
+
+ @Override
+ void onChecksumError(String algorithm, ChecksumKind kind,
ChecksumFailureException exception) throws ChecksumFailureException {
+ delegate.onChecksumError(algorithm, kind, exception)
+ }
+
+ /**
+ * Accepts the download when no checksum could be validated.
+ *
+ * <p>This is the single point where the policy departs from the
wrapped {@code fail}
+ * policy. It covers both a repository that publishes no checksum and
one whose checksum
+ * could not be retrieved, since Maven Resolver reports a failed
retrieval through
+ * {@link #onChecksumError} and then arrives here having validated
nothing.
+ */
+ @Override
+ void onNoMoreChecksums() {
+ LOG.debug('No checksum could be validated for {}{}; accepting the
artifact unverified',
+ resource.repositoryUrl, resource.resourceName)
+ }
Review Comment:
As implemented, `onNoMoreChecksums()` will also accept artifacts when
checksum retrieval failed (i.e., `onChecksumError` was invoked but no checksum
was ultimately validated). That weakens integrity guarantees beyond “repository
publishes no checksums”. Consider tracking whether `onChecksumError(...)`
occurred in this wrapper and, if so, delegate to the underlying fail behavior
(or rethrow) in `onNoMoreChecksums()`, while only tolerating the true ‘no
checksums published’ path.
##########
subprojects/groovy-grape-maven/src/test/groovy/groovy/grape/maven/GrapeMavenTest.groovy:
##########
@@ -150,6 +150,46 @@ ${depsXml}
assert !uris.any { it.toString().contains('dep-optional-1.0.0.jar') }
}
+ @Test
+ void testArtifactWithoutChecksumsResolves() {
+ File repoDir = new
File(Files.createTempDirectory('grape-maven-nochecksum-test').toFile(), 'repo')
+
+ String g = 'dev.grape.nochecksum'
+ deleteCachedGroup(g)
+ publishArtifact(repoDir, g, 'plain', '1.0.0')
+
+ Grape.addResolver(name: 'local-nochecksum-test', root:
repoDir.toURI().toString(), m2Compatible: true)
+ URI[] uris = Grape.resolve([autoDownload: true, classLoader: new
GroovyClassLoader()],
+ [groupId: g, artifactId: 'plain', version: '1.0.0'])
+
+ // A repository that publishes no .sha1/.md5 must still resolve,
matching the Ivy engine.
+ assert uris.any { it.toString().contains('plain-1.0.0.jar') }
+ }
+
+ @Test
+ void testArtifactWithMismatchedChecksumIsRejected() {
+ File repoDir = new
File(Files.createTempDirectory('grape-maven-badchecksum-test').toFile(), 'repo')
+
+ String g = 'dev.grape.badchecksum'
+ deleteCachedGroup(g)
+ publishArtifact(repoDir, g, 'tampered', '1.0.0')
+
+ // Publish a checksum that does not describe the jar, as a tampered
mirror would.
+ File artifactDir = new File(repoDir, g.replace('.', '/') +
'/tampered/1.0.0')
+ new File(artifactDir, 'tampered-1.0.0.jar.sha1').text = '0' * 40
+
+ Grape.addResolver(name: 'local-badchecksum-test', root:
repoDir.toURI().toString(), m2Compatible: true)
+ def ex = shouldFail {
+ Grape.resolve([autoDownload: true, classLoader: new
GroovyClassLoader()],
+ [groupId: g, artifactId: 'tampered', version: '1.0.0'])
+ }
+
+ // Ensure it failed for the checksum, not for some unrelated
resolution problem.
+ StringWriter trace = new StringWriter()
+ ex.printStackTrace(new PrintWriter(trace))
+ assert trace.toString().toLowerCase().contains('checksum')
Review Comment:
Asserting on the rendered stack trace text is brittle (message
wording/casing can change, and stack traces are not a stable contract). A more
robust test is to assert on exception types in the causal chain (e.g., that a
`ChecksumFailureException` is present) or on a specific, structured property if
available.
> GrapeMaven: fail resolution on checksum mismatch
> ------------------------------------------------
>
> Key: GROOVY-12265
> URL: https://issues.apache.org/jira/browse/GROOVY-12265
> Project: Groovy
> Issue Type: Improvement
> Components: Grape
> Reporter: Paul King
> Priority: Major
>
> The default of mismatch failing by default should be similar to Ivy. Maven
> would also fail when no checksums are present whereas Ivy only fails for a
> bad checksum.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
