Allowed special characters on path or filename (#39)

Author: umbreak

build.sbt

@@ -26,15 +26,15 @@ scalafmt: {
 
 // Dependency versions
 val akkaVersion           = "2.6.0"
-val akkaHttpVersion       = "10.1.10"
+val akkaHttpVersion       = "10.1.11"
 val apacheCompressVersion = "1.19"
 val alpakkaVersion        = "1.1.2"
-val catsVersion           = "2.0.0"
+val catsVersion           = "2.1.0"
 val catsEffectVersion     = "2.0.0"
 val circeVersion          = "0.12.3"
 val commonsVersion        = "0.20.0"
 val iamVersion            = "1.2.0+16-382dc073"
-val mockitoVersion        = "1.7.1"
+val mockitoVersion        = "1.10.1"
 val monixVersion          = "3.1.0"
 val pureconfigVersion     = "0.12.1"
 val scalaTestVersion      = "3.1.0"

src/main/scala/ch/epfl/bluebrain/nexus/storage/Storages.scala

@@ -1,5 +1,6 @@
 package ch.epfl.bluebrain.nexus.storage
 
+import java.net.URLDecoder
 import java.nio.file.StandardCopyOption._
 import java.nio.file.{Files, Path, Paths}
 import java.security.MessageDigest
@@ -121,13 +122,16 @@ object Storages {
       F: Effect[F]
   ) extends Storages[F, AkkaSource] {
 
+    private def decode(path: Uri.Path): String =
+      Try(URLDecoder.decode(path.toString, "UTF-8")).getOrElse(path.toString())
+
     private def basePath(name: String, protectedDir: Boolean = true): Path = {
       val path = config.rootVolume.resolve(name).normalize()
       if (protectedDir) path.resolve(config.protectedDirectory).normalize() else path
     }
 
     private def filePath(name: String, relativePath: Uri.Path, protectedDir: Boolean = true): Path =
-      basePath(name, protectedDir).resolve(Paths.get(relativePath.toString())).normalize()
+      basePath(name, protectedDir).resolve(Paths.get(decode(relativePath))).normalize()
 
     def exists(name: String): BucketExistence = {
       val path = basePath(name)
@@ -156,7 +160,7 @@ object Storages {
                 case (digFuture, ioFuture) =>
                   digFuture.zipWith(ioFuture) {
                     case (digest, io) if absFilePath.toFile.exists() =>
-                      Future(FileAttributes(s"file://$absFilePath", io.count, digest, detectMediaType(absFilePath)))
+                      Future(FileAttributes(absFilePath.toAkkaUri, io.count, digest, detectMediaType(absFilePath)))
                     case _ =>
                       Future.failed(InternalError(s"I/O error writing file to path '$relativeFilePath'"))
                   }
@@ -196,7 +200,7 @@ object Storages {
             F.fromTry(Try(Files.createDirectories(absDestPath.getParent))) >>
             F.fromTry(Try(Files.move(absSourcePath, absDestPath, ATOMIC_MOVE))) >>
             F.pure(cache.asyncComputePut(absDestPath, digestConfig.algorithm)) >>
-            F.pure(Right(FileAttributes(s"file://$absDestPath", computedSize, Digest.empty, mediaType)))
+            F.pure(Right(FileAttributes(absDestPath.toAkkaUri, computedSize, Digest.empty, mediaType)))
         }
       }
 

src/main/scala/ch/epfl/bluebrain/nexus/storage/attributes/AttributesCacheActor.scala

@@ -5,15 +5,16 @@ import java.time.Clock
 
 import akka.NotUsed
 import akka.actor.{Actor, ActorLogging, ActorSystem, Props}
+import akka.http.scaladsl.model.MediaTypes.`application/octet-stream`
 import akka.stream.javadsl.Sink
 import akka.stream.scaladsl.{Flow, Keep, Source}
 import akka.stream.{OverflowStrategy, QueueOfferResult}
 import cats.effect.Effect
 import cats.effect.implicits._
 import ch.epfl.bluebrain.nexus.storage.File.{Digest, FileAttributes}
+import ch.epfl.bluebrain.nexus.storage._
 import ch.epfl.bluebrain.nexus.storage.attributes.AttributesCacheActor.Protocol._
 import ch.epfl.bluebrain.nexus.storage.config.AppConfig.DigestConfig
-import akka.http.scaladsl.model.MediaTypes.`application/octet-stream`
 
 import scala.collection.mutable
 import scala.concurrent.duration._
@@ -114,7 +115,7 @@ class AttributesCacheActor[F[_]: Effect, S](computation: AttributesComputation[F
   }
 
   private def emptyAttributes(path: Path) =
-    FileAttributes(s"file://$path", 0L, Digest.empty, `application/octet-stream`)
+    FileAttributes(path.toAkkaUri, 0L, Digest.empty, `application/octet-stream`)
 
   private def removeOldest(n: Int) =
     map --= map.take(n).keySet

src/main/scala/ch/epfl/bluebrain/nexus/storage/attributes/AttributesComputation.scala

@@ -4,16 +4,16 @@ import java.nio.file.{Files, Path}
 import java.security.MessageDigest
 
 import akka.http.scaladsl.model.HttpCharsets.`UTF-8`
-import akka.http.scaladsl.model.{ContentType, MediaType, MediaTypes}
 import akka.http.scaladsl.model.MediaTypes.{`application/octet-stream`, `application/x-tar`}
+import akka.http.scaladsl.model.{ContentType, MediaType, MediaTypes}
 import akka.stream.Materializer
-import cats.implicits._
 import akka.stream.scaladsl.{Keep, Sink}
 import akka.util.ByteString
 import cats.effect.Effect
+import cats.implicits._
 import ch.epfl.bluebrain.nexus.storage.File.{Digest, FileAttributes}
 import ch.epfl.bluebrain.nexus.storage.StorageError.InternalError
-import ch.epfl.bluebrain.nexus.storage.{fileSource, folderSource, AkkaSource}
+import ch.epfl.bluebrain.nexus.storage._
 import org.apache.commons.io.FilenameUtils
 
 import scala.concurrent.{ExecutionContext, Future}
@@ -84,7 +84,7 @@ object AttributesComputation {
               .alsoToMat(sinkSize)(Keep.right)
               .toMat(sinkDigest(msgDigest)) { (bytesF, digestF) =>
                 (bytesF, digestF).mapN {
-                  case (bytes, digest) => FileAttributes(s"file://$path", bytes, digest, detectMediaType(path, isDir))
+                  case (bytes, digest) => FileAttributes(path.toAkkaUri, bytes, digest, detectMediaType(path, isDir))
                 }
               }
               .run()

src/main/scala/ch/epfl/bluebrain/nexus/storage/package.scala

@@ -69,6 +69,14 @@ package object storage {
       */
     def descendantOf(parent: JavaPath): Boolean =
       inner(parent, path.getParent)
+
+    /**
+      * Converts a Java Path to an Akka [[Uri]]
+      */
+    def toAkkaUri: Uri = {
+      val pathString = path.toUri.toString
+      if (pathString.endsWith("/")) Uri(pathString.dropRight(1)) else Uri(pathString)
+    }
   }
 
   /**

src/test/scala/ch/epfl/bluebrain/nexus/storage/DiskStorageSpec.scala

@@ -153,7 +153,7 @@ class DiskStorageSpec
 
       "fail when call to nexus-fixer fails" in new AbsoluteDirectoryCreated {
         val badStorage   = new DiskStorage[IO](sConfig.copy(fixerCommand = List("/bin/false")), dConfig, cache)
-        val file         = "some/folder/myfile.txt"
+        val file         = "some/folder/my !file.txt"
         val absoluteFile = baseRootPath.resolve(Paths.get(file.toString))
         Files.createDirectories(absoluteFile.getParent)
         Files.write(absoluteFile, "something".getBytes(StandardCharsets.UTF_8))
@@ -180,22 +180,22 @@ class DiskStorageSpec
       }
 
       "fail when destination already exists" in new AbsoluteDirectoryCreated {
-        val file         = "some/folder/myfile.txt"
+        val file         = "some/folder/my !file.txt"
         val absoluteFile = baseRootPath.resolve(Paths.get(file.toString))
         Files.createDirectories(absoluteFile.getParent)
         Files.write(absoluteFile, "something".getBytes(StandardCharsets.UTF_8))
 
-        val fileDest = basePath.resolve(Paths.get("myfile.txt"))
+        val fileDest = basePath.resolve(Paths.get("my !file.txt"))
         Files.write(fileDest, "something".getBytes(StandardCharsets.UTF_8))
         storage
-          .moveFile(name, Uri.Path(file), Uri.Path("myfile.txt"))
+          .moveFile(name, Uri.Path(file), Uri.Path("my !file.txt"))
           .rejected[PathAlreadyExists] shouldEqual
-          PathAlreadyExists(name, Uri.Path("myfile.txt"))
+          PathAlreadyExists(name, Uri.Path("my !file.txt"))
       }
 
       "fail when destination is out of bucket scope" in new AbsoluteDirectoryCreated {
-        val file         = "some/folder/myfile.txt"
-        val dest         = Uri.Path("../some/other.txt")
+        val file         = "some/folder/my !file.txt"
+        val dest         = Uri.Path("../some/other path.txt")
         val absoluteFile = baseRootPath.resolve(Paths.get(file.toString))
         Files.createDirectories(absoluteFile.getParent)
 
@@ -208,25 +208,25 @@ class DiskStorageSpec
       }
 
       "pass on file" in new AbsoluteDirectoryCreated {
-        val file         = "some/folder/myfile.txt"
+        val file         = "some/folder/my !file.txt"
         val absoluteFile = baseRootPath.resolve(Paths.get(file.toString))
         Files.createDirectories(absoluteFile.getParent)
 
         val content = "some content"
         Files.write(absoluteFile, content.getBytes(StandardCharsets.UTF_8))
 
-        storage.moveFile(name, Uri.Path(file), Uri.Path("some/other.txt")).accepted shouldEqual
-          FileAttributes(s"file://${basePath.resolve("some/other.txt")}", 12L, Digest.empty, `text/plain(UTF-8)`)
+        storage.moveFile(name, Uri.Path(file), Uri.Path("some/other path.txt")).accepted shouldEqual
+          FileAttributes(s"file://${basePath.resolve("some/other%20path.txt")}", 12L, Digest.empty, `text/plain(UTF-8)`)
         Files.exists(absoluteFile) shouldEqual false
-        Files.exists(basePath.resolve("some/other.txt")) shouldEqual true
+        Files.exists(basePath.resolve("some/other path.txt")) shouldEqual true
       }
 
       "pass on directory" in new AbsoluteDirectoryCreated {
         val dir         = "some/folder"
         val absoluteDir = baseRootPath.resolve(Paths.get(dir.toString))
         Files.createDirectories(absoluteDir)
 
-        val absoluteFile = absoluteDir.resolve(Paths.get("myfile.txt"))
+        val absoluteFile = absoluteDir.resolve(Paths.get("my !file.txt"))
         val content      = "some content"
         Files.write(absoluteFile, content.getBytes(StandardCharsets.UTF_8))
 
@@ -236,7 +236,7 @@ class DiskStorageSpec
         Files.exists(absoluteDir) shouldEqual false
         Files.exists(absoluteFile) shouldEqual false
         Files.exists(resolvedDir) shouldEqual true
-        Files.exists(basePath.resolve("some/other/myfile.txt")) shouldEqual true
+        Files.exists(basePath.resolve("some/other/my !file.txt")) shouldEqual true
       }
     }
 

src/test/scala/ch/epfl/bluebrain/nexus/storage/attributes/AttributesCacheSpec.scala

@@ -8,6 +8,7 @@ import akka.actor.ActorSystem
 import akka.testkit.TestKit
 import akka.util.Timeout
 import ch.epfl.bluebrain.nexus.commons.test.Randomness
+import ch.epfl.bluebrain.nexus.storage._
 import ch.epfl.bluebrain.nexus.storage.File.{Digest, FileAttributes}
 import ch.epfl.bluebrain.nexus.storage.config.AppConfig.DigestConfig
 import monix.eval.Task
@@ -47,7 +48,7 @@ class AttributesCacheSpec
     val path: Path                      = Paths.get(genString())
     val digest                          = Digest(config.algorithm, genString())
     val attributes                      = FileAttributes(s"file://$path", genInt().toLong, digest, `image/jpeg`)
-    def attributesEmpty(p: Path = path) = FileAttributes(s"file://$p", 0L, Digest.empty, `application/octet-stream`)
+    def attributesEmpty(p: Path = path) = FileAttributes(p.toAkkaUri, 0L, Digest.empty, `application/octet-stream`)
     val counter                         = new AtomicInteger(0)
 
     implicit val clock: Clock = new Clock {
@@ -81,12 +82,9 @@ class AttributesCacheSpec
 
     "verify 2 concurrent computations" in new Ctx {
       val list = List.tabulate(10) { i =>
-        Paths.get(i.toString) -> FileAttributes(
-          s"file://$i",
-          i.toLong,
-          Digest(config.algorithm, i.toString),
-          `image/jpeg`
-        )
+        val path   = Paths.get(i.toString)
+        val digest = Digest(config.algorithm, i.toString)
+        path -> FileAttributes(path.toAkkaUri, i.toLong, digest, `image/jpeg`)
       }
       val time = System.currentTimeMillis()
 
@@ -120,12 +118,9 @@ class AttributesCacheSpec
 
     "verify remove oldest" in new Ctx {
       val list = List.tabulate(20) { i =>
-        Paths.get(i.toString) -> FileAttributes(
-          s"file://$i",
-          i.toLong,
-          Digest(config.algorithm, i.toString),
-          `image/jpeg`
-        )
+        val path   = Paths.get(i.toString)
+        val digest = Digest(config.algorithm, i.toString)
+        path -> FileAttributes(path.toAkkaUri, i.toLong, digest, `image/jpeg`)
       }
 
       forAll(list) {
@@ -150,12 +145,9 @@ class AttributesCacheSpec
 
     "verify failure is skipped" in new Ctx {
       val list = List.tabulate(5) { i =>
-        Paths.get(i.toString) -> FileAttributes(
-          s"file://$i",
-          i.toLong,
-          Digest(config.algorithm, i.toString),
-          `image/jpeg`
-        )
+        val path   = Paths.get(i.toString)
+        val digest = Digest(config.algorithm, i.toString)
+        path -> FileAttributes(path.toAkkaUri, i.toLong, digest, `image/jpeg`)
       }
 
       forAll(list) {