From ff20ab3623c1b8b99c5d87df32c1148a5a6708a1 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Fri, 4 Mar 2016 00:41:07 +0100 Subject: [PATCH] Rework checksum calculation a bit To handle those starting with zeros in particular --- cache/src/main/scala/coursier/Cache.scala | 98 +++++++++------- .../abc/test/0.1/test-0.1.pom} | 0 .../abc/test/0.1/test-0.1.pom.md5} | 0 .../abc/test/0.1/test-0.1.pom.sha1} | 0 .../1.0.0-M9/coursier_2.11-1.0.0-M9.pom | 83 ++++++++++++++ .../1.0.0-M9/coursier_2.11-1.0.0-M9.pom.md5 | 1 + .../1.0.0-M9/coursier_2.11-1.0.0-M9.pom.sha1 | 1 + .../scala/coursier/test/ChecksumTests.scala | 105 ++++++++++-------- 8 files changed, 201 insertions(+), 87 deletions(-) rename tests/jvm/src/test/resources/checksums/abc.com/{test.jar => com/abc/test/0.1/test-0.1.pom} (100%) rename tests/jvm/src/test/resources/checksums/abc.com/{test.jar.md5 => com/abc/test/0.1/test-0.1.pom.md5} (100%) rename tests/jvm/src/test/resources/checksums/abc.com/{test.jar.sha1 => com/abc/test/0.1/test-0.1.pom.sha1} (100%) create mode 100644 tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom create mode 100644 tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.md5 create mode 100644 tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.sha1 diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index 3652a0970..b23c29038 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -1,10 +1,12 @@ package coursier +import java.math.BigInteger import java.net.{ HttpURLConnection, URL, URLConnection } import java.nio.channels.{ OverlappingFileLockException, FileLock } import java.nio.file.{ StandardCopyOption, Files => NioFiles } import java.security.MessageDigest -import java.util.concurrent.{ConcurrentHashMap, Executors, ExecutorService} +import java.util.concurrent.{ ConcurrentHashMap, Executors, ExecutorService } +import java.util.regex.Pattern import coursier.ivy.IvyRepository @@ -442,15 +444,18 @@ object Cache { Nondeterminism[Task].gather(tasks) } - def parseChecksum(content: String): Option[String] = { - // matches md5 or sha1 - val pattern = "^[0-9a-f]{32}([0-9a-f]{8})?" + // matches md5 or sha1 + private val checksumPattern = Pattern.compile("^[0-9a-f]{32}([0-9a-f]{8})?") + + def parseChecksum(content: String): Option[BigInteger] = content .linesIterator .toStream .map(_.toLowerCase.replaceAll("\\s", "")) - .find(_.matches(pattern)) - } + .collectFirst { + case rawSum if checksumPattern.matcher(rawSum).matches() => + new BigInteger(rawSum, 16) + } def validateChecksum( artifact: Artifact, @@ -469,44 +474,56 @@ object Cache { artifact0.checksumUrls.get(sumType) match { case Some(sumFile) => Task { - val sum = parseChecksum( - new String(NioFiles.readAllBytes(new File(sumFile).toPath), "UTF-8")) - .getOrElse("") + val sumOpt = parseChecksum( + new String(NioFiles.readAllBytes(new File(sumFile).toPath), "UTF-8") + ) - val f = new File(artifact0.url) - val md = MessageDigest.getInstance(sumType) - val is = new FileInputStream(f) - val res = try { - var lock: FileLock = null - try { - lock = is.getChannel.tryLock(0L, Long.MaxValue, true) - if (lock == null) - -\/(FileError.Locked(f)) - else { - withContent(is, md.update(_, 0, _)) - \/-(()) + sumOpt match { + case None => + FileError.ChecksumFormatError(sumType, sumFile).left + + case Some(sum) => + val f = new File(artifact0.url) + val md = MessageDigest.getInstance(sumType) + val is = new FileInputStream(f) + val res = try { + var lock: FileLock = null + try { + lock = is.getChannel.tryLock(0L, Long.MaxValue, true) + if (lock == null) + FileError.Locked(f).left + else { + withContent(is, md.update(_, 0, _)) + ().right + } + } + catch { + case e: OverlappingFileLockException => + FileError.Locked(f).left + } + finally if (lock != null) lock.release() + } finally is.close() + + res.flatMap { _ => + val digest = md.digest() + val calculatedSum = new BigInteger(1, digest) + + if (sum == calculatedSum) + ().right + else + FileError.WrongChecksum( + sumType, + calculatedSum.toString(16), + sum.toString(16), + artifact0.url, + sumFile + ).left } - } - catch { - case e: OverlappingFileLockException => - -\/(FileError.Locked(f)) - } - finally if (lock != null) lock.release() - } finally is.close() - - res.flatMap { _ => - val digest = md.digest() - val calculatedSum = f"${BigInt(1, digest)}%x" - - if (sum == calculatedSum) - \/-(()) - else - -\/(FileError.WrongChecksum(sumType, calculatedSum, sum, artifact0.url, sumFile)) } } case None => - Task.now(-\/(FileError.ChecksumNotFound(sumType, artifact0.url))) + Task.now(FileError.ChecksumNotFound(sumType, artifact0.url).left) } } } @@ -693,6 +710,11 @@ object FileError { file: String ) extends FileError(s"$sumType checksum not found: $file") + final case class ChecksumFormatError( + sumType: String, + file: String + ) extends FileError(s"Unrecognized $sumType checksum format in $file") + final case class WrongChecksum( sumType: String, got: String, diff --git a/tests/jvm/src/test/resources/checksums/abc.com/test.jar b/tests/jvm/src/test/resources/checksums/abc.com/com/abc/test/0.1/test-0.1.pom similarity index 100% rename from tests/jvm/src/test/resources/checksums/abc.com/test.jar rename to tests/jvm/src/test/resources/checksums/abc.com/com/abc/test/0.1/test-0.1.pom diff --git a/tests/jvm/src/test/resources/checksums/abc.com/test.jar.md5 b/tests/jvm/src/test/resources/checksums/abc.com/com/abc/test/0.1/test-0.1.pom.md5 similarity index 100% rename from tests/jvm/src/test/resources/checksums/abc.com/test.jar.md5 rename to tests/jvm/src/test/resources/checksums/abc.com/com/abc/test/0.1/test-0.1.pom.md5 diff --git a/tests/jvm/src/test/resources/checksums/abc.com/test.jar.sha1 b/tests/jvm/src/test/resources/checksums/abc.com/com/abc/test/0.1/test-0.1.pom.sha1 similarity index 100% rename from tests/jvm/src/test/resources/checksums/abc.com/test.jar.sha1 rename to tests/jvm/src/test/resources/checksums/abc.com/com/abc/test/0.1/test-0.1.pom.sha1 diff --git a/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom b/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom new file mode 100644 index 000000000..a4798d782 --- /dev/null +++ b/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom @@ -0,0 +1,83 @@ + + + 4.0.0 + com.github.alexarchambault + coursier_2.11 + jar + coursier + https://github.com/alexarchambault/coursier + 1.0.0-M9 + + + Apache 2.0 + http://opensource.org/licenses/Apache-2.0 + repo + + + coursier + + com.github.alexarchambault + https://github.com/alexarchambault/coursier + + + scm:git:github.com/alexarchambault/coursier.git + scm:git:git@github.com:alexarchambault/coursier.git + github.com/alexarchambault/coursier.git + + + + alexarchambault + Alexandre Archambault + https://github.com/alexarchambault + + + + + org.scala-lang + scala-library + 2.11.7 + + + org.scoverage + scalac-scoverage-runtime_2.11 + 1.1.0 + provided + + + org.scoverage + scalac-scoverage-plugin_2.11 + 1.1.0 + provided + + + org.scalaz + scalaz-core_2.11 + 7.1.2 + + + org.scala-lang.modules + scala-xml_2.11 + 1.0.3 + + + + + sonatypereleases + sonatype-releases + https://oss.sonatype.org/content/repositories/releases/ + default + + + ScalazBintrayRepo + Scalaz Bintray Repo + http://dl.bintray.com/scalaz/releases/ + default + + + sonatypereleases + sonatype-releases + https://oss.sonatype.org/content/repositories/releases/ + default + + + \ No newline at end of file diff --git a/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.md5 b/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.md5 new file mode 100644 index 000000000..edfb87536 --- /dev/null +++ b/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.md5 @@ -0,0 +1 @@ +3eca59042e315b331708226fe8506824 \ No newline at end of file diff --git a/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.sha1 b/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.sha1 new file mode 100644 index 000000000..3e0671365 --- /dev/null +++ b/tests/jvm/src/test/resources/checksums/abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom.sha1 @@ -0,0 +1 @@ +0dcc0ee5f3e9f3ede7712f1fab14021c6e07a657 \ No newline at end of file diff --git a/tests/jvm/src/test/scala/coursier/test/ChecksumTests.scala b/tests/jvm/src/test/scala/coursier/test/ChecksumTests.scala index d542aaf16..f48875a18 100644 --- a/tests/jvm/src/test/scala/coursier/test/ChecksumTests.scala +++ b/tests/jvm/src/test/scala/coursier/test/ChecksumTests.scala @@ -2,72 +2,79 @@ package coursier package test import java.io.File +import java.math.BigInteger import java.util.concurrent.Executors import utest._ +import scalaz.concurrent.Strategy + object ChecksumTests extends TestSuite { val tests = TestSuite { - val junkSha1 = - """./spark-core_2.11/1.2.0/spark-core_2.11-1.2.0.pom: -5630 42A5 4B97 E31A F452 9EA0 DB79 BA2C 4C2B B6CC - """.stripMargin + 'parse - { + // https://repo1.maven.org/maven2/org/apache/spark/spark-core_2.11/1.2.0/spark-core_2.11-1.2.0.pom.sha1 + // as of 2016-03-02 + val junkSha1 = + "./spark-core_2.11/1.2.0/spark-core_2.11-1.2.0.pom:\n" + + "5630 42A5 4B97 E31A F452 9EA0 DB79 BA2C 4C2B B6CC" - val cleanSha1 = - """563042a54b97e31af4529ea0db79ba2c4c2bb6cc - """.stripMargin + val cleanSha1 = "563042a54b97e31af4529ea0db79ba2c4c2bb6cc" - val checksum = Some("563042a54b97e31af4529ea0db79ba2c4c2bb6cc") + val checksum = Some(new BigInteger(cleanSha1, 16)) - 'parseJunkChecksum{ - assert(Cache.parseChecksum(junkSha1) == checksum) + 'junk - { + assert(Cache.parseChecksum(junkSha1) == checksum) + } + + 'clean - { + assert(Cache.parseChecksum(cleanSha1) == checksum) + } } - 'parseCleanChecksum{ - assert(Cache.parseChecksum(cleanSha1) == checksum) - } + 'artifact - { + val cachePath = getClass.getResource("/checksums").getPath - val artifact = Artifact( - "http://abc.com/test.jar", - Map ( - "MD5" -> "http://abc.com/test.jar.md5", - "SHA-1" -> "http://abc.com/test.jar.sha1" - ), - Map.empty, - Attributes("jar"), - changing = false - ) - - val pool = Executors.newFixedThreadPool(1) - val cachePath = getClass.getResource("/checksums").getPath - - val cache = Seq( - "http://" -> new File(cachePath), - "https://" -> new File(cachePath) - ) - - 'sha1{ - val res = Cache.validateChecksum( - artifact, - "SHA-1", - cache, - pool - ) - assert(res.run.run.isRight) - } - - 'md5{ - val res = Cache.validateChecksum( - artifact, - "MD5", - cache, - pool + val cache = Seq( + "http://" -> new File(cachePath), + "https://" -> new File(cachePath) ) - assert(res.run.run.isRight) + def validate(artifact: Artifact, sumType: String) = + Cache.validateChecksum( + artifact, + sumType, + cache, + Strategy.DefaultExecutorService + ).run.run + + def artifact(url: String) = Artifact( + url, + Map( + "MD5" -> (url + ".md5"), + "SHA-1" -> (url + ".sha1") + ), + Map.empty, + Attributes("jar"), + changing = false + ) + + val artifacts = Seq( + "http://abc.com/com/abc/test/0.1/test-0.1.pom", + // corresponding SHA-1 starts with a 0 + "http://abc.com/com/github/alexarchambault/coursier_2.11/1.0.0-M9/coursier_2.11-1.0.0-M9.pom" + ).map(artifact) + + def validateAll(sumType: String) = + for (artifact <- artifacts) { + val res = validate(artifact, sumType) + assert(res.isRight) + } + + 'sha1 - validateAll("SHA-1") + 'md5 - validateAll("MD5") } } } \ No newline at end of file