From 2c9e012c96e8ca23a45b068c877b88ddc5bbba95 Mon Sep 17 00:00:00 2001
From: eikek <eike.kettner@posteo.de>
Date: Thu, 7 Jul 2022 11:31:36 +0200
Subject: [PATCH] Fix url parsing with trailing slash

Refs: #1545
---
 .../docspell/analysis/contact/Contact.scala   |  2 +-
 .../scala/docspell/common/LenientUri.scala    | 20 ++++++++++---------
 .../scala/docspell/common/UrlMatcher.scala    |  2 +-
 .../docspell/common/LenientUriTest.scala      |  7 +++++++
 .../docspell/store/file/FileUrlReader.scala   |  3 ++-
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/modules/analysis/src/main/scala/docspell/analysis/contact/Contact.scala b/modules/analysis/src/main/scala/docspell/analysis/contact/Contact.scala
index 2a9f87a9..cbdf0522 100644
--- a/modules/analysis/src/main/scala/docspell/analysis/contact/Contact.scala
+++ b/modules/analysis/src/main/scala/docspell/analysis/contact/Contact.scala
@@ -50,7 +50,7 @@ object Contact {
       p match {
         case LenientUri.RootPath  => false
         case LenientUri.EmptyPath => false
-        case LenientUri.NonEmptyPath(segs) =>
+        case LenientUri.NonEmptyPath(segs, _) =>
           Ident.fromString(segs.last).isRight &&
           segs.init.takeRight(3) == List("open", "upload", "item")
       }
diff --git a/modules/common/src/main/scala/docspell/common/LenientUri.scala b/modules/common/src/main/scala/docspell/common/LenientUri.scala
index 4061969d..c9f797ab 100644
--- a/modules/common/src/main/scala/docspell/common/LenientUri.scala
+++ b/modules/common/src/main/scala/docspell/common/LenientUri.scala
@@ -121,7 +121,7 @@ object LenientUri {
     val isRoot = true
     val isEmpty = false
     def /(seg: String): Path =
-      NonEmptyPath(NonEmptyList.of(seg))
+      NonEmptyPath(NonEmptyList.of(seg), false)
     def asString = "/"
   }
   case object EmptyPath extends Path {
@@ -129,20 +129,22 @@ object LenientUri {
     val isRoot = false
     val isEmpty = true
     def /(seg: String): Path =
-      NonEmptyPath(NonEmptyList.of(seg))
+      NonEmptyPath(NonEmptyList.of(seg), false)
     def asString = ""
   }
-  case class NonEmptyPath(segs: NonEmptyList[String]) extends Path {
+  case class NonEmptyPath(segs: NonEmptyList[String], trailingSlash: Boolean)
+      extends Path {
     def segments = segs.toList
     val isEmpty = false
     val isRoot = false
+    private val slashSuffix = if (trailingSlash) "/" else ""
     def /(seg: String): Path =
       copy(segs = segs.append(seg))
     def asString =
       segs.head match {
-        case "."  => segments.map(percentEncode).mkString("/")
-        case ".." => segments.map(percentEncode).mkString("/")
-        case _    => "/" + segments.map(percentEncode).mkString("/")
+        case "."  => segments.map(percentEncode).mkString("/") + slashSuffix
+        case ".." => segments.map(percentEncode).mkString("/") + slashSuffix
+        case _    => "/" + segments.map(percentEncode).mkString("/") + slashSuffix
       }
   }
 
@@ -157,14 +159,14 @@ object LenientUri {
       str.trim match {
         case "/" => Right(RootPath)
         case ""  => Right(EmptyPath)
-        case _ =>
+        case uriStr =>
           Either.fromOption(
-            stripLeading(str, '/')
+            stripLeading(uriStr, '/')
               .split('/')
               .toList
               .traverse(percentDecode)
               .flatMap(NonEmptyList.fromList)
-              .map(NonEmptyPath.apply),
+              .map(NonEmptyPath(_, uriStr.endsWith("/"))),
             s"Invalid path: $str"
           )
       }
diff --git a/modules/common/src/main/scala/docspell/common/UrlMatcher.scala b/modules/common/src/main/scala/docspell/common/UrlMatcher.scala
index c8fd393e..de978dd9 100644
--- a/modules/common/src/main/scala/docspell/common/UrlMatcher.scala
+++ b/modules/common/src/main/scala/docspell/common/UrlMatcher.scala
@@ -62,7 +62,7 @@ object UrlMatcher {
       // strip path to only match prefixes
       val mPath: LenientUri.Path =
         NonEmptyList.fromList(url.path.segments.take(pathSegmentCount)) match {
-          case Some(nel) => LenientUri.NonEmptyPath(nel)
+          case Some(nel) => LenientUri.NonEmptyPath(nel, false)
           case None      => LenientUri.RootPath
         }
 
diff --git a/modules/common/src/test/scala/docspell/common/LenientUriTest.scala b/modules/common/src/test/scala/docspell/common/LenientUriTest.scala
index 4bd69582..a81e7452 100644
--- a/modules/common/src/test/scala/docspell/common/LenientUriTest.scala
+++ b/modules/common/src/test/scala/docspell/common/LenientUriTest.scala
@@ -29,4 +29,11 @@ class LenientUriTest extends FunSuite {
     )
     assertEquals(LenientUri.percentDecode("a%25b%5Cc%7Cd%23e"), "a%b\\c|d#e".some)
   }
+
+  test("parse with trailing slash") {
+    assertEquals(LenientUri.unsafe("http://a.com/").asString, "http://a.com/")
+    assertEquals(LenientUri.unsafe("http://a.com").asString, "http://a.com")
+    assertEquals(LenientUri.unsafe("http://a.com/path").asString, "http://a.com/path")
+    assertEquals(LenientUri.unsafe("http://a.com/path/").asString, "http://a.com/path/")
+  }
 }
diff --git a/modules/store/src/main/scala/docspell/store/file/FileUrlReader.scala b/modules/store/src/main/scala/docspell/store/file/FileUrlReader.scala
index df69f421..47b8cc52 100644
--- a/modules/store/src/main/scala/docspell/store/file/FileUrlReader.scala
+++ b/modules/store/src/main/scala/docspell/store/file/FileUrlReader.scala
@@ -24,7 +24,8 @@ object FileUrlReader {
       scheme = Nel.of(scheme),
       authority = Some(""),
       path = LenientUri.NonEmptyPath(
-        Nel.of(key.collective.id, key.category.id.id, key.id.id)
+        Nel.of(key.collective.id, key.category.id.id, key.id.id),
+        false
       ),
       query = None,
       fragment = None