michael-o commented on a change in pull request #139:
URL: https://github.com/apache/maven-resolver/pull/139#discussion_r792996067
##########
File path: src/site/markdown/about-checksums.md
##########
@@ -0,0 +1,72 @@
+# About checksums
Review comment:
About Checksums
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java
##########
@@ -42,6 +42,11 @@
import org.eclipse.aether.impl.RepositoryEventDispatcher;
import org.eclipse.aether.internal.impl.DefaultTrackingFileManager;
import org.eclipse.aether.internal.impl.TrackingFileManager;
+import org.eclipse.aether.internal.impl.checksum.ChecksumAlgorithmFactoryMD5;
Review comment:
I think the also name should be at the of the class name. Just like
`FileInputStream` and not `InputStreamFile`.
##########
File path: src/site/site.xml
##########
@@ -27,6 +27,7 @@ under the License.
<menu name="Overview">
<item name="Introduction" href="index.html"/>
<item name="Configuration" href="configuration.html"/>
+ <item name="About checksums" href="about-checksums.html"/>
Review comment:
About Checksums
##########
File path:
maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/ChecksumValidator.java
##########
@@ -241,12 +252,12 @@ public void commit()
{
if ( tmp instanceof File )
{
- fileProcessor.move( (File) tmp, checksumFile );
+ fileProcessor.move( ( File ) tmp, checksumFile );
Review comment:
As far as I know we don't have spaces around casts.
##########
File path: src/site/markdown/about-checksums.md
##########
@@ -0,0 +1,72 @@
+# About checksums
+<!--
+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.
+-->
+
+Maven Resolver uses checksums to verify the consistency of the downloaded
+artifacts. Checksums are usually laid out in repositories next to the file in
question, with file
+extension telling the checksum algorithm that produced the given checksum file
content. Current Maven Repository
+layout contains SHA-1 and MD5 checksums by default (they are produced by
Resolver by default).
+
+Historically, Maven Resolver used `java.security.MessageDigest` to implement
checksums. So to say, secure one-way
+hashes provided by Java Cryptography Architecture were (mis)used to implement
checksums for transport integrity
+validation. There is no misunderstanding here, secure hashes MAY be used as
checksums, as there is quite some
+overlap between "checksums" and "hashes" in general. But this simplicity comes
at a price: cryptographically "safe"
+algorithms require way more CPU cycles to calculate checksum, while all their
purpose is just
+"integrity validation", nothing more. There is no "security", "trust" or
whatever else implied or expected from
+them.
+
+If you are interested in "trust" in your artifacts, it is Artifact Signatures
(for example
+[GPG Signatures](https://maven.apache.org/plugins/maven-gpg-plugin/)) that you
should look for.
+
+Hence, the usual argument that "XXX algorithm is unsafe, deprecated, not
secure anymore" does not stand in use case
+of Maven Resolver: there is nothing "secure" being involved with checksums.
Moreover, this is true not only for SHA-1
+algorithm, but even for its "elder brother" MD5. Both algorithms are still
widely used today as "transport integrity
+validation" or "error detection" (aka "bit-rot detection").
+
+## Checksum changes
Review comment:
Checksum Changes
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
##########
@@ -163,45 +204,47 @@ public URI getLocation( Metadata metadata, boolean upload
)
return toUri( path.toString() );
}
- public List<Checksum> getChecksums( Artifact artifact, boolean upload,
URI location )
+ @Override
+ public List<ChecksumLocation> getChecksumLocations( Artifact artifact,
boolean upload, URI location )
{
return getChecksums( location );
}
- public List<Checksum> getChecksums( Metadata metadata, boolean upload,
URI location )
+ @Override
+ public List<ChecksumLocation> getChecksumLocations( Metadata metadata,
boolean upload, URI location )
{
return getChecksums( location );
}
- private List<Checksum> getChecksums( URI location )
+ private List<ChecksumLocation> getChecksums( URI location )
Review comment:
`getChecksumLocations`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
