fix(server:files): more security and bug fixes

actor-server/actor-fs-adapters/src/main/scala/im/actor/server/file/FileStorageAdapter.scala

@@ -14,7 +14,7 @@ object FileStorageAdapter {
 trait FileStorageAdapter extends UploadActions with DownloadActions with UploadKeyParsing
 
 final case class UnsafeFileName(fileName: String) {
-  lazy val safe: String = new File(fileName).toPath.normalize().getFileName.toString
+  lazy val safe: String = new File(fileName.replace("\u0000", "")).toPath.normalize().getFileName.toString
 }
 
 private[file] trait UploadActions {

actor-server/actor-fs-adapters/src/main/scala/im/actor/server/file/local/FileStorageOperations.scala

@@ -3,6 +3,8 @@ package im.actor.server.file.local
 import akka.actor.ActorSystem
 import akka.event.Logging
 import better.files.{ File, _ }
+import im.actor.server.db.DbExtension
+import im.actor.server.persist.FileRepo
 
 import scala.concurrent.{ ExecutionContext, Future, blocking }
 
@@ -13,6 +15,7 @@ trait FileStorageOperations extends LocalUploadKeyImplicits {
   protected val storageLocation: String
 
   private lazy val log = Logging(system, getClass)
+  private lazy val db = DbExtension(system).db
 
   protected def createFile(fileId: Long, name: String, file: File): Future[File] =
     Future(file.copyTo(getOrCreateFileDir(fileId) / getFileName(name)))
@@ -65,11 +68,14 @@ trait FileStorageOperations extends LocalUploadKeyImplicits {
   protected def deleteUploadedParts(dir: File, partNames: Seq[String]): Future[Unit] =
     Future.sequence(partNames map { part ⇒ Future((dir / part).delete()) }) map (_ ⇒ ())
 
-  protected def getFile(fileId: Long, optName: Option[String]): Future[File] =
-    getFile(fileId, optName getOrElse "")
+  protected def getFile(fileId: Long): Future[File] =
+    db.run(FileRepo.find(fileId)) flatMap {
+      case Some(model) ⇒ getFile(fileId, model.name)
+      case None        ⇒ Future.failed(new RuntimeException("File not found")) // TODO: throw an exception convertable to 404 response
+    }
 
-  protected def getFile(fileId: Long, name: String): Future[File] =
-    Future(fileDirectory(fileId) / getFileName(name))
+  protected def getFile(fileId: Long, fileName: String): Future[File] =
+    Future(fileDirectory(fileId) / getFileName(fileName))
 
   protected def getFileName(name: String) = if (name.trim.isEmpty) "file" else name
 

actor-server/actor-fs-adapters/src/main/scala/im/actor/server/file/local/LocalFileStorageAdapter.scala

@@ -2,6 +2,7 @@ package im.actor.server.file.local
 
 import java.io
 import java.io.IOException
+import java.net.URLEncoder
 import java.time.{ Duration, Instant }
 
 import akka.actor.ActorSystem
@@ -137,7 +138,7 @@ final class LocalFileStorageAdapter(_system: ActorSystem)
    */
   override def getFileDownloadUrl(file: model.File, accessHash: Long): Future[Option[String]] = {
     if (ACLFiles.fileAccessHash(file.id, file.accessSalt) == accessHash) {
-      val filePart = Option(file.name) filter (_.trim.nonEmpty) map (n ⇒ s"/$n") getOrElse ""
+      val filePart = Option(file.name) filter (_.trim.nonEmpty) map (n ⇒ s"/${URLEncoder.encode(n, "UTF-8")}") getOrElse ""
       val query = baseUri
         .withPath(Uri.Path(s"/v1/files/${file.id}" + filePart))
         .withQuery(Uri.Query("expires" → expiresAt().toString))

actor-server/actor-fs-adapters/src/main/scala/im/actor/server/file/local/http/FilesHttpHandler.scala

@@ -50,10 +50,9 @@ private[local] final class FilesHttpHandler(storageConfig: LocalFileStorageConfi
               //v1/files/:fileId/:fileName
               //v1/files/:fileId
               path(Segments(0, 1)) { seqName =>
-                val optName = seqName.headOption
-                log.debug("Download file request, fileId: {}, fileName: {}", fileId, optName)
+                log.debug("Download file request, fileId: {}", fileId)
                 withRangeSupport {
-                  onComplete(getFile(fileId, optName)) {
+                  onComplete(getFile(fileId)) {
                     case Success(file) =>
                       log.debug("Serving file: {} parts", fileId)
                       complete(file.loadBytes)

actor-server/actor-tests/src/test/scala/im/actor/server/api/rpc/service/FilesServiceSpec.scala

@@ -93,7 +93,7 @@ final class FilesServiceSpec
   }
 
   def completeUpload() = {
-    whenReady(service.handleCommitFileUpload(uploadKey, "/etc/passwd/The.Filë%00 – 'Fear and Loathing in Las Vegas'")) { resp ⇒
+    whenReady(service.handleCommitFileUpload(uploadKey, "/etc/passwd/The.Filë%00\u0000 – 'Fear and Loathing in Las Vegas'")) { resp ⇒
       resp should matchPattern {
         case Ok(ResponseCommitFileUpload(_)) ⇒
       }
@@ -111,7 +111,7 @@ final class FilesServiceSpec
       resp.toOption.get.url
     }
 
-    urlStr should include("The.Fil%C3%AB%00%20%E2%80%93%20'Fear%20and%20Loathing%20in%20Las%20Vegas'?")
+    urlStr should include("The.Fil%C3%AB%2500+%E2%80%93+'Fear+and+Loathing+in+Las+Vegas'?")
     urlStr shouldNot include("//The")
     urlStr shouldNot include("etc")
     urlStr shouldNot include("passwd")