From 9917c0eb5400b8db7343193d0c295f65a16217f8 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 22 Aug 2016 00:18:37 +0200 Subject: [PATCH 1/4] Explicitly close HTTP connections --- cache/src/main/scala/coursier/Cache.scala | 242 ++++++++++++++-------- 1 file changed, 150 insertions(+), 92 deletions(-) diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index 4f61a32be..aa7d2f595 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -22,6 +22,7 @@ import java.io.{ Serializable => _, _ } import scala.concurrent.duration.{ Duration, DurationInt } import scala.util.Try +import scala.util.control.NonFatal trait AuthenticatedURLConnection extends URLConnection { def authenticate(authentication: Authentication): Unit @@ -325,27 +326,41 @@ object Cache { def referenceFileExists: Boolean = referenceFileOpt.exists(_.exists()) def urlConn(url0: String) = { - val conn = url(url0).openConnection() // FIXME Should this be closed? - // Dummy user-agent instead of the default "Java/...", - // so that we are not returned incomplete/erroneous metadata - // (Maven 2 compatibility? - happens for snapshot versioning metadata, - // this is SO FSCKING CRAZY) - conn.setRequestProperty("User-Agent", "") + var conn: URLConnection = null - for (auth <- artifact.authentication) - conn match { - case authenticated: AuthenticatedURLConnection => - authenticated.authenticate(auth) - case conn0: HttpURLConnection => - conn0.setRequestProperty( - "Authorization", - "Basic " + basicAuthenticationEncode(auth.user, auth.password) - ) - case _ => + try { + conn = url(url0).openConnection() // FIXME Should this be closed? + // Dummy user-agent instead of the default "Java/...", + // so that we are not returned incomplete/erroneous metadata + // (Maven 2 compatibility? - happens for snapshot versioning metadata, + // this is SO FSCKING CRAZY) + conn.setRequestProperty("User-Agent", "") + + for (auth <- artifact.authentication) + conn match { + case authenticated: AuthenticatedURLConnection => + authenticated.authenticate(auth) + case conn0: HttpURLConnection => + conn0.setRequestProperty( + "Authorization", + "Basic " + basicAuthenticationEncode(auth.user, auth.password) + ) + case _ => // FIXME Authentication is ignored - } + } - conn + conn + } catch { + case NonFatal(e) => + if (conn != null) + conn match { + case conn0: HttpURLConnection => + conn0.getInputStream.close() + conn0.disconnect() + case _ => + } + throw e + } } @@ -369,34 +384,47 @@ object Cache { ): EitherT[Task, FileError, Option[Long]] = EitherT { Task { - urlConn(url) match { - case c: HttpURLConnection => - logger.foreach(_.checkingUpdates(url, currentLastModifiedOpt)) + var conn: URLConnection = null - var success = false - try { - c.setRequestMethod("HEAD") - val remoteLastModified = c.getLastModified + try { + conn = urlConn(url) - // TODO 404 Not found could be checked here + conn match { + case c: HttpURLConnection => + logger.foreach(_.checkingUpdates(url, currentLastModifiedOpt)) - val res = - if (remoteLastModified > 0L) - Some(remoteLastModified) - else - None + var success = false + try { + c.setRequestMethod("HEAD") + val remoteLastModified = c.getLastModified - success = true - logger.foreach(_.checkingUpdatesResult(url, currentLastModifiedOpt, res)) + // TODO 404 Not found could be checked here - res.right - } finally { - if (!success) - logger.foreach(_.checkingUpdatesResult(url, currentLastModifiedOpt, None)) + val res = + if (remoteLastModified > 0L) + Some(remoteLastModified) + else + None + + success = true + logger.foreach(_.checkingUpdatesResult(url, currentLastModifiedOpt, res)) + + res.right + } finally { + if (!success) + logger.foreach(_.checkingUpdatesResult(url, currentLastModifiedOpt, None)) + } + + case other => + -\/(FileError.DownloadError(s"Cannot do HEAD request with connection $other ($url)")) + } + } finally { + if (conn != null) + conn match { + case conn0: HttpURLConnection => + conn0.disconnect() + case _ => } - - case other => - -\/(FileError.DownloadError(s"Cannot do HEAD request with connection $other ($url)")) } } } @@ -512,59 +540,70 @@ object Cache { val alreadyDownloaded = tmp.length() - val conn0 = urlConn(url) + var conn: URLConnection = null - val (partialDownload, conn) = conn0 match { - case conn0: HttpURLConnection if alreadyDownloaded > 0L => - conn0.setRequestProperty("Range", s"bytes=$alreadyDownloaded-") + try { + conn = urlConn(url) - if (conn0.getResponseCode == partialContentResponseCode) { - val ackRange = Option(conn0.getHeaderField("Content-Range")).getOrElse("") + val partialDownload = conn match { + case conn0: HttpURLConnection if alreadyDownloaded > 0L => + conn0.setRequestProperty("Range", s"bytes=$alreadyDownloaded-") - if (ackRange.startsWith(s"bytes $alreadyDownloaded-")) - (true, conn0) - else - // unrecognized Content-Range header -> start a new connection with no resume - (false, urlConn(url)) - } else - (false, conn0) + (conn0.getResponseCode == partialContentResponseCode) && { + val ackRange = Option(conn0.getHeaderField("Content-Range")).getOrElse("") - case _ => (false, conn0) - } - - if (responseCode(conn) == Some(404)) - FileError.NotFound(url, permanent = Some(true)).left - else if (responseCode(conn) == Some(401)) - FileError.Unauthorized(url, realm = realm(conn)).left - else { - for (len0 <- Option(conn.getContentLengthLong) if len0 >= 0L) { - val len = len0 + (if (partialDownload) alreadyDownloaded else 0L) - logger.foreach(_.downloadLength(url, len, alreadyDownloaded)) - } - - val in = new BufferedInputStream(conn.getInputStream, bufferSize) - - val result = - try { - val out = withStructureLock(cache) { - tmp.getParentFile.mkdirs() - new FileOutputStream(tmp, partialDownload) + ackRange.startsWith(s"bytes $alreadyDownloaded-") || { + // unrecognized Content-Range header -> start a new connection with no resume + conn0.getInputStream.close() + conn0.disconnect() + conn = urlConn(url) + false + } } - try readFullyTo(in, out, logger, url, if (partialDownload) alreadyDownloaded else 0L) - finally out.close() - } finally in.close() - - withStructureLock(cache) { - file.getParentFile.mkdirs() - NioFiles.move(tmp.toPath, file.toPath, StandardCopyOption.ATOMIC_MOVE) + case _ => false } - for (lastModified <- Option(conn.getLastModified) if lastModified > 0L) - file.setLastModified(lastModified) + if (responseCode(conn) == Some(404)) + FileError.NotFound(url, permanent = Some(true)).left + else if (responseCode(conn) == Some(401)) + FileError.Unauthorized(url, realm = realm(conn)).left + else { + for (len0 <- Option(conn.getContentLengthLong) if len0 >= 0L) { + val len = len0 + (if (partialDownload) alreadyDownloaded else 0L) + logger.foreach(_.downloadLength(url, len, alreadyDownloaded)) + } - doTouchCheckFile(file) + val in = new BufferedInputStream(conn.getInputStream, bufferSize) - result.right + val result = + try { + val out = withStructureLock(cache) { + tmp.getParentFile.mkdirs() + new FileOutputStream(tmp, partialDownload) + } + try readFullyTo(in, out, logger, url, if (partialDownload) alreadyDownloaded else 0L) + finally out.close() + } finally in.close() + + withStructureLock(cache) { + file.getParentFile.mkdirs() + NioFiles.move(tmp.toPath, file.toPath, StandardCopyOption.ATOMIC_MOVE) + } + + for (lastModified <- Option(conn.getLastModified) if lastModified > 0L) + file.setLastModified(lastModified) + + doTouchCheckFile(file) + + result.right + } + } finally { + if (conn != null) + conn match { + case conn0: HttpURLConnection => + conn0.disconnect() + case _ => + } } } } @@ -852,16 +891,35 @@ object Cache { pool = pool, ttl = ttl ).leftMap(_.describe).flatMap { f => - val res = if (!f.isDirectory && f.exists()) { - Try(new String(NioFiles.readAllBytes(f.toPath), "UTF-8")) match { - case scala.util.Success(content) => - // stripping any UTF-8 BOM if any, see - // https://github.com/alexarchambault/coursier/issues/316 and the corresponding test - Right(content.stripPrefix(utf8Bom)) - case scala.util.Failure(e) => + + def notFound(f: File) = Left(s"${f.getCanonicalPath} not found") + + def read(f: File) = + try Right(new String(NioFiles.readAllBytes(f.toPath), "UTF-8").stripPrefix(utf8Bom)) + catch { + case NonFatal(e) => Left(s"Could not read (file:${f.getCanonicalPath}): ${e.getMessage}") } - } else Left(s"Could not read (file:${f.getCanonicalPath}) (isFile:${!f.isDirectory}) (exists:${f.exists()})") + + val res = if (f.exists()) { + if (f.isDirectory) { + if (artifact.url.startsWith("file:")) + Left("Not implemented: listing of local directories") + else { + val f0 = new File(f, ".directory") + + if (f0.exists()) { + if (f0.isDirectory) + Left(s"Woops: ${f.getCanonicalPath} is a directory") + else + read(f0) + } else + notFound(f0) + } + } else + read(f) + } else + notFound(f) EitherT.fromEither(Task.now[Either[String, String]](res)) } From bbd367d14ee189f9b699caecca52645d4b239327 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 22 Aug 2016 00:18:40 +0200 Subject: [PATCH 2/4] Have coursierDependencyTree not trigger fetching dependency JARs --- plugin/src/main/scala-2.10/coursier/Tasks.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/scala-2.10/coursier/Tasks.scala b/plugin/src/main/scala-2.10/coursier/Tasks.scala index d3c010bba..f700ced85 100644 --- a/plugin/src/main/scala-2.10/coursier/Tasks.scala +++ b/plugin/src/main/scala-2.10/coursier/Tasks.scala @@ -953,7 +953,7 @@ object Tasks { coursierResolution }.value - val config = classpathConfiguration.value.name + val config = configuration.value.name val configs = coursierConfigurations.value val includedConfigs = configs.getOrElse(config, Set.empty) + config From c17cf67734cf7d0bbedae2588e1203c5e348b409 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 22 Aug 2016 00:19:49 +0200 Subject: [PATCH 3/4] More reliable fallback dependencies Should fix the jrudolph/sbt-dependency-graph case of https://gist.github.com/paulp/62eaca1850904137959ad9121cce6b15 --- cache/src/main/scala/coursier/Cache.scala | 29 ++++++++-- .../coursier/core/compatibility/package.scala | 7 ++- .../coursier/core/compatibility/package.scala | 24 ++++++--- .../scala/coursier/ivy/IvyRepository.scala | 6 +-- .../FallbackDependenciesRepository.scala | 53 +++++++++++++------ .../sbt-coursier/from-wrong-url/build.sbt | 7 +++ .../from-wrong-url/project/plugins.sbt | 11 ++++ .../from-wrong-url/src/main/scala/Main.scala | 13 +++++ .../sbt-test/sbt-coursier/from-wrong-url/test | 3 ++ 9 files changed, 121 insertions(+), 32 deletions(-) create mode 100644 plugin/src/sbt-test/sbt-coursier/from-wrong-url/build.sbt create mode 100644 plugin/src/sbt-test/sbt-coursier/from-wrong-url/project/plugins.sbt create mode 100644 plugin/src/sbt-test/sbt-coursier/from-wrong-url/src/main/scala/Main.scala create mode 100644 plugin/src/sbt-test/sbt-coursier/from-wrong-url/test diff --git a/cache/src/main/scala/coursier/Cache.scala b/cache/src/main/scala/coursier/Cache.scala index aa7d2f595..90e815fc4 100644 --- a/cache/src/main/scala/coursier/Cache.scala +++ b/cache/src/main/scala/coursier/Cache.scala @@ -903,9 +903,32 @@ object Cache { val res = if (f.exists()) { if (f.isDirectory) { - if (artifact.url.startsWith("file:")) - Left("Not implemented: listing of local directories") - else { + if (artifact.url.startsWith("file:")) { + + val elements = f.listFiles().map { c => + val name = c.getName + val name0 = if (c.isDirectory) + name + "/" + else + name + + s"""
  • $name0
    • """ + }.mkString + + val page = + s""" + | + | + | + |
      + |$elements + |
    + | + | + """.stripMargin + + Right(page) + } else { val f0 = new File(f, ".directory") if (f0.exists()) { diff --git a/core/js/src/main/scala/coursier/core/compatibility/package.scala b/core/js/src/main/scala/coursier/core/compatibility/package.scala index eebc41fa1..3b8659b5b 100644 --- a/core/js/src/main/scala/coursier/core/compatibility/package.scala +++ b/core/js/src/main/scala/coursier/core/compatibility/package.scala @@ -93,7 +93,12 @@ package object compatibility { def encodeURIComponent(s: String): String = g.encodeURIComponent(s).asInstanceOf[String] - def listWebPageSubDirectories(page: String): Seq[String] = { + def listWebPageSubDirectories(url: String, page: String): Seq[String] = { + // TODO + ??? + } + + def listWebPageFiles(url: String, page: String): Seq[String] = { // TODO ??? } diff --git a/core/jvm/src/main/scala/coursier/core/compatibility/package.scala b/core/jvm/src/main/scala/coursier/core/compatibility/package.scala index 11f052fe2..d4fbbd74c 100644 --- a/core/jvm/src/main/scala/coursier/core/compatibility/package.scala +++ b/core/jvm/src/main/scala/coursier/core/compatibility/package.scala @@ -56,17 +56,25 @@ package object compatibility { def encodeURIComponent(s: String): String = new java.net.URI(null, null, null, -1, s, null, null) .toASCIIString - def listWebPageSubDirectories(page: String): Seq[String] = + def listWebPageDirectoryElements(url: String, page: String, directories: Boolean): Seq[String] = Jsoup.parse(page) - .select("a[href~=[^/]*/]") + .select("a") .asScala .toVector - .map { elem => - elem - .attr("href") - .stripPrefix(":") // bintray typically prepends these - .stripSuffix("/") + .map(_.attr("href")) + .collect { + case elem if elem.nonEmpty && elem.endsWith("/") == directories => + elem + .stripSuffix("/") + .stripPrefix(url) + .stripPrefix(":") // bintray typically prepends these } - .filter(n => n != "." && n != "..") + .filter(n => !n.contains("/") && n != "." && n != "..") + + def listWebPageSubDirectories(url: String, page: String): Seq[String] = + listWebPageDirectoryElements(url, page, directories = true) + + def listWebPageFiles(url: String, page: String): Seq[String] = + listWebPageDirectoryElements(url, page, directories = false) } diff --git a/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala b/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala index fd3d562b2..d10a9ae7f 100644 --- a/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala +++ b/core/shared/src/main/scala/coursier/ivy/IvyRepository.scala @@ -147,8 +147,8 @@ case class IvyRepository( s"Don't know how to list revisions of ${metadataPattern.string}".left } - def fromWebPage(s: String) = { - val subDirs = coursier.core.compatibility.listWebPageSubDirectories(s) + def fromWebPage(url: String, s: String) = { + val subDirs = coursier.core.compatibility.listWebPageSubDirectories(url, s) val versions = subDirs.map(Parse.version).collect { case Some(v) => v } val versionsInItv = versions.filter(itv.contains) @@ -173,7 +173,7 @@ case class IvyRepository( for { url <- EitherT(F.point(listingUrl)) s <- fetch(artifactFor(url)) - res <- fromWebPage(s) + res <- fromWebPage(url, s) } yield res } } diff --git a/plugin/src/main/scala-2.10/coursier/FallbackDependenciesRepository.scala b/plugin/src/main/scala-2.10/coursier/FallbackDependenciesRepository.scala index b2d495970..08018ffe7 100644 --- a/plugin/src/main/scala-2.10/coursier/FallbackDependenciesRepository.scala +++ b/plugin/src/main/scala-2.10/coursier/FallbackDependenciesRepository.scala @@ -8,7 +8,7 @@ case class FallbackDependenciesRepository( fallbacks: Map[(Module, String), (URL, Boolean)] ) extends Repository { - private val source = new Artifact.Source { + private val source: Artifact.Source = new Artifact.Source { def artifacts( dependency: Dependency, project: Project, @@ -35,22 +35,41 @@ case class FallbackDependenciesRepository( EitherT.left(F.point("No fallback URL found")) case Some((url, _)) => - val proj = Project( - module, - version, - Nil, - Map.empty, - None, - Nil, - Nil, - Nil, - None, - None, - None, - Nil, - Info.empty - ) - EitherT.right(F.point((source, proj))) + val urlStr = url.toExternalForm + val idx = urlStr.lastIndexOf('/') + + if (idx < 0 || urlStr.endsWith("/")) + EitherT.left(F.point(s"$url doesn't point to a file")) + else { + val (dirUrlStr, fileName) = urlStr.splitAt(idx + 1) + + fetch(Artifact(dirUrlStr, Map.empty, Map.empty, Attributes("", ""), changing = true, None)).flatMap { listing => + + val files = coursier.core.compatibility.listWebPageFiles(dirUrlStr, listing) + + if (files.contains(fileName)) { + + val proj = Project( + module, + version, + Nil, + Map.empty, + None, + Nil, + Nil, + Nil, + None, + None, + None, + Nil, + Info.empty + ) + + EitherT.right(F.point((source, proj))) + } else + EitherT.left(F.point(s"$fileName not found under $dirUrlStr")) + } + } } } diff --git a/plugin/src/sbt-test/sbt-coursier/from-wrong-url/build.sbt b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/build.sbt new file mode 100644 index 000000000..fbb442536 --- /dev/null +++ b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/build.sbt @@ -0,0 +1,7 @@ +scalaVersion := "2.11.8" + +// keeping the default cache policies here + +libraryDependencies += "com.chuusai" %% "shapeless" % "2.3.2" from { + "https://repo1.maven.org/maven2/com/chuusai/shapeless_2.11/2.3.242/shapeless_2.11-2.3.242.jar" +} diff --git a/plugin/src/sbt-test/sbt-coursier/from-wrong-url/project/plugins.sbt b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/project/plugins.sbt new file mode 100644 index 000000000..152225a9e --- /dev/null +++ b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/project/plugins.sbt @@ -0,0 +1,11 @@ +{ + val pluginVersion = sys.props.getOrElse( + "plugin.version", + throw new RuntimeException( + """|The system property 'plugin.version' is not defined. + |Specify this property using the scriptedLaunchOpts -D.""".stripMargin + ) + ) + + addSbtPlugin("io.get-coursier" % "sbt-coursier" % pluginVersion) +} diff --git a/plugin/src/sbt-test/sbt-coursier/from-wrong-url/src/main/scala/Main.scala b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/src/main/scala/Main.scala new file mode 100644 index 000000000..c9401ff3e --- /dev/null +++ b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/src/main/scala/Main.scala @@ -0,0 +1,13 @@ +import java.io.File +import java.nio.file.Files + +import shapeless._ + +object Main extends App { + case class CC(s: String) + val cc = CC("OK") + val l = Generic[CC].to(cc) + val msg = l.head + + Files.write(new File("output").toPath, msg.getBytes("UTF-8")) +} diff --git a/plugin/src/sbt-test/sbt-coursier/from-wrong-url/test b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/test new file mode 100644 index 000000000..2182f57b0 --- /dev/null +++ b/plugin/src/sbt-test/sbt-coursier/from-wrong-url/test @@ -0,0 +1,3 @@ +$ delete output +> run +$ exists output From 8ef5a4b46dd26434b72f39ce2b57c60b729ec056 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 22 Aug 2016 00:19:52 +0200 Subject: [PATCH 4/4] Use sub-project names rather than their coordinates --- .../src/main/scala-2.10/coursier/Tasks.scala | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/plugin/src/main/scala-2.10/coursier/Tasks.scala b/plugin/src/main/scala-2.10/coursier/Tasks.scala index f700ced85..069b63726 100644 --- a/plugin/src/main/scala-2.10/coursier/Tasks.scala +++ b/plugin/src/main/scala-2.10/coursier/Tasks.scala @@ -276,9 +276,6 @@ object Tasks { Module(scalaOrganization, "scalap") -> scalaVersion ) - private def projectDescription(project: Project) = - s"${project.module.organization}:${project.module.name}:${project.version}" - private def createLogger() = new TermDisplay(new OutputStreamWriter(System.err)) private lazy val globalPluginPattern = { @@ -325,6 +322,8 @@ object Tasks { lazy val cm = coursierSbtClassifiersModule.value + lazy val projectName = thisProjectRef.value.project + val (currentProject, fallbackDependencies) = if (sbtClassifiers) { val sv = scalaVersion.value @@ -614,8 +613,7 @@ object Tasks { if (verbosityLevel >= 0) log.info( - s"Updating ${projectDescription(currentProject)}" + - (if (sbtClassifiers) " (sbt classifiers)" else "") + s"Updating $projectName" + (if (sbtClassifiers) " (sbt classifiers)" else "") ) if (verbosityLevel >= 2) for (depRepr <- depsRepr(currentProject.dependencies)) @@ -663,7 +661,7 @@ object Tasks { } if (verbosityLevel >= 0) - log.info(s"Resolved ${projectDescription(currentProject)} dependencies") + log.info(s"Resolved $projectName dependencies") res } finally { @@ -705,6 +703,8 @@ object Tasks { lazy val cm = coursierSbtClassifiersModule.value + lazy val projectName = thisProjectRef.value.project + val currentProject = if (sbtClassifiers) { val sv = scalaVersion.value @@ -834,7 +834,7 @@ object Tasks { if (verbosityLevel >= 0) log.info( - s"Fetching artifacts of ${projectDescription(currentProject)}" + + s"Fetching artifacts of $projectName" + (if (sbtClassifiers) " (sbt classifiers)" else "") ) @@ -850,7 +850,7 @@ object Tasks { if (verbosityLevel >= 0) log.info( - s"Fetched artifacts of ${projectDescription(currentProject)}" + + s"Fetched artifacts of $projectName" + (if (sbtClassifiers) " (sbt classifiers)" else "") ) @@ -925,6 +925,8 @@ object Tasks { ignoreArtifactErrors: Boolean = false ) = Def.task { + lazy val projectName = thisProjectRef.value.project + val currentProject = if (sbtClassifiers) { val cm = coursierSbtClassifiersModule.value @@ -968,7 +970,7 @@ object Tasks { // use sbt logging? println( - projectDescription(currentProject) + "\n" + + projectName + "\n" + Print.dependencyTree(dependencies0, subRes, printExclusions = true, inverse) ) }