Repository: tipsy/javalin
Pull request #1262 (2021-05-30 20:39:32)

Set default path to root URL
This PR sets "/" as default path when removing cookies. 
Please see [issue 1257](https://github.com/tipsy/javalin/issues/1257) for details.

A new test class has been added as I could not see any existing test class where the test cases would fit. 


tipsy: Great work @madsleegiil! There are a lot of cookie tests in `TestRequest` and `TestResponse`, it would be nice if you could also move them to `TestCookies`
madsleegiil: 


> Great work @madsleegiil! There are a lot of cookie tests in `TestRequest` and `TestResponse`, it would be nice if you could also move them to `TestCookies`

I have moved the test cases that I thought was relevant. Please point out if any more are missing. 
I also corrected the tests I wrote as they did not considered the path when asserting on removed cookies. 
tipsy: Nice! Could you please revert the accidental formatting changes?
madsleegiil: Sorry for not having been more careful with the automatic formatting changes. 
Will the PR be squash merged if approved? If not, do you prefer that I clean up the commit history on the branch myself?
tipsy: > Sorry for not having been more careful with the automatic formatting changes.

Not problem at all. The change looks great! 

> Will the PR be squash merged if approved? If not, do you prefer that I clean up the commit history on the branch myself?

You don't have to clean anything up, I have this button: 
![image](https://user-images.githubusercontent.com/1521451/120205703-8e7e7800-c22a-11eb-9219-d792b82688ee.png)

I'll click it once the tests pass :)

Patch:
diff --git a/javalin/src/main/java/io/javalin/http/Context.kt b/javalin/src/main/java/io/javalin/http/Context.kt
index afbbb2f4..82ab1c2c 100644
--- a/javalin/src/main/java/io/javalin/http/Context.kt
+++ b/javalin/src/main/java/io/javalin/http/Context.kt
@@ -424,7 +424,7 @@ open class Context(@JvmField val req: HttpServletRequest, @JvmField val res: Htt
 
     /** Removes cookie specified by name and path (optional). */
     @JvmOverloads
-    fun removeCookie(name: String, path: String? = null): Context {
+    fun removeCookie(name: String, path: String? = "/"): Context {
         res.addCookie(Cookie(name, "").apply {
             this.path = path
             this.maxAge = 0


Test patch:
diff --git a/javalin/src/test/java/io/javalin/TestCookie.kt b/javalin/src/test/java/io/javalin/TestCookie.kt
new file mode 100644
index 00000000..47804c08
--- /dev/null
+++ b/javalin/src/test/java/io/javalin/TestCookie.kt
@@ -0,0 +1,131 @@
+package io.javalin
+
+import com.mashape.unirest.http.Unirest
+import io.javalin.core.util.Header
+import io.javalin.testing.TestUtil
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Test
+import javax.servlet.http.Cookie
+
+class TestCookie {
+
+    @Test
+    fun `cookie set on unspecified path is set to root URL`() = TestUtil.test { app, http ->
+        app.get("/cookie") { it.cookie(Cookie("key", "value").apply { path = "/" }) }
+
+        val setCookieResponse = http.get("/cookie")
+        val cookiePath = setCookieResponse.headers.getFirst(Header.SET_COOKIE).split(";")[1].replaceFirst(" ", "")
+
+        assertThat(cookiePath).isEqualTo("Path=/")
+    }
+
+    @Test
+    fun `removing cookie without specifying path will remove cookie set to root URL`() = TestUtil.test { app, http ->
+        val cookie = Cookie("key", "value").apply {
+            path = "/"
+        }
+        app.get("/cookie") { it.cookie(cookie) }
+        app.get("/cookie-remove") { it.removeCookie(cookie.name) }
+        val setCookieResponse = http.get("/cookie")
+        assertThat(setCookieResponse.headers.getFirst(Header.SET_COOKIE)).isEqualTo("key=value; Path=/")
+
+        val removeCookieResponse = http.get("/cookie-remove")
+
+        assertThat(cookieIsEffectivelyRemoved(removeCookieResponse.headers.getFirst(Header.SET_COOKIE), "/")).isTrue
+    }
+
+    @Test
+    fun `removing cookie without specifying path will not remove cookie set to non-root URL`() = TestUtil.test { app, http ->
+        val cookie = Cookie("key", "value").apply {
+            path = "/some-path"
+        }
+        app.get("/cookie") { it.cookie(cookie) }
+        app.get("/cookie-remove") { it.removeCookie(cookie.name) }
+        val setCookieResponse = http.get("/cookie")
+        assertThat(setCookieResponse.headers.getFirst(Header.SET_COOKIE)).isEqualTo("key=value; Path=/some-path")
+
+        val removeCookieResponse = http.get("/cookie-remove")
+
+        assertThat(cookieIsEffectivelyRemoved(removeCookieResponse.headers.getFirst(Header.SET_COOKIE), "/some-path")).isFalse
+    }
+
+    /*
+     * Requests
+     */
+    @Test
+    fun `single cookie returns null when missing`() = TestUtil.test { app, http ->
+        app.get("/read-cookie-1") { ctx -> ctx.result("" + ctx.cookie("my-cookie")) }
+        assertThat(http.getBody("/read-cookie-1")).isEqualTo("null")
+    }
+
+    @Test
+    fun `single cookie works`() = TestUtil.test { app, http ->
+        app.get("/read-cookie-2") { ctx -> ctx.result(ctx.cookie("my-cookie")!!) }
+        val response = Unirest.get("${http.origin}/read-cookie-2").header(Header.COOKIE, "my-cookie=my-cookie-value").asString()
+        assertThat(response.body).isEqualTo("my-cookie-value")
+    }
+
+    @Test
+    fun `cookie-map returns empty when no cookies are set`() = TestUtil.test { app, http ->
+        app.get("/read-cookie-3") { ctx -> ctx.result(ctx.cookieMap().toString()) }
+        assertThat(http.getBody("/read-cookie-3")).isEqualTo("{}")
+    }
+
+    @Test
+    fun `cookie-map returns all cookies if cookies are set`() = TestUtil.test { app, http ->
+        app.get("/read-cookie-4") { ctx -> ctx.result(ctx.cookieMap().toString()) }
+        val response = Unirest.get("${http.origin}/read-cookie-4").header(Header.COOKIE, "k1=v1;k2=v2;k3=v3").asString()
+        assertThat(response.body).isEqualTo("{k1=v1, k2=v2, k3=v3}")
+    }
+
+    /*
+     * Responses
+     */
+    @Test
+    fun `setting a cookie works`() = TestUtil.test { app, http ->
+        app.get("/create-cookie") { ctx -> ctx.cookie("Test", "Tast") }
+        app.get("/get-cookie") { ctx -> ctx.result(ctx.cookie("Test")!!) }
+        assertThat(http.get("/create-cookie").headers.getFirst(Header.SET_COOKIE)).isEqualTo("Test=Tast; Path=/")
+        assertThat(http.getBody("/get-cookie")).isEqualTo("Tast")
+    }
+
+    @Test
+    fun `setting a Cookie object works`() = TestUtil.test { app, http ->
+        app.get("/create-cookie") { ctx -> ctx.cookie(Cookie("Hest", "Hast").apply { maxAge = 7 }) }
+        assertThat(http.get("/create-cookie").headers.getFirst(Header.SET_COOKIE)).contains("Hest=Hast")
+        assertThat(http.get("/create-cookie").headers.getFirst(Header.SET_COOKIE)).contains("Max-Age=7")
+    }
+
+    @Test
+    fun `can't set duplicate cookies when other cookies set`() = TestUtil.test { app, http ->
+        app.get("/create-cookies") {
+            it.cookie("Test-1", "1")
+            it.cookie("Test-2", "2")
+            it.cookie("Test-3", "3")
+            it.cookie("Test-3", "4")  // duplicate
+        }
+        val response = http.get("/create-cookies");
+        assertThat(response.headers[Header.SET_COOKIE]!!).contains("Test-1=1; Path=/")
+        assertThat(response.headers[Header.SET_COOKIE]!!).contains("Test-2=2; Path=/")
+        assertThat(response.headers[Header.SET_COOKIE]!!).contains("Test-3=4; Path=/")
+        assertThat(response.headers[Header.SET_COOKIE]!!.size).isEqualTo(3)
+    }
+
+    @Test
+    fun `can't set duplicate cookies when no other cookies set`() = TestUtil.test { app, http ->
+        app.get("/create-cookies") {
+            it.cookie("MyCookie", "A")
+            it.cookie("MyCookie", "B")  // duplicate
+        }
+        val response = http.get("/create-cookies");
+        assertThat(response.headers[Header.SET_COOKIE]!!).contains("MyCookie=B; Path=/")
+        assertThat(response.headers[Header.SET_COOKIE]!!.size).isEqualTo(1)
+    }
+
+    private fun cookieIsEffectivelyRemoved(cookie: String, path: String): Boolean {
+        val pathMatches = cookie.split(";")[1].split("=")[1] == path
+        val expiresEpoch = cookie.split(";")[2] == " Expires=Thu, 01-Jan-1970 00:00:00 GMT"
+        val maxAgeZero = cookie.split(";")[3] == " Max-Age=0"
+        return pathMatches && expiresEpoch && maxAgeZero
+    }
+}
diff --git a/javalin/src/test/java/io/javalin/TestRequest.kt b/javalin/src/test/java/io/javalin/TestRequest.kt
index a787e872..236cb519 100644
--- a/javalin/src/test/java/io/javalin/TestRequest.kt
+++ b/javalin/src/test/java/io/javalin/TestRequest.kt
@@ -109,35 +109,6 @@ class TestRequest {
         assertThat(http.getBody("/attr-map")).contains("hest=hast")
     }
 
-    /*
-     * Cookies
-     */
-    @Test
-    fun `single cookie returns null when missing`() = TestUtil.test { app, http ->
-        app.get("/read-cookie-1") { ctx -> ctx.result("" + ctx.cookie("my-cookie")) }
-        assertThat(http.getBody("/read-cookie-1")).isEqualTo("null")
-    }
-
-    @Test
-    fun `single cookie works`() = TestUtil.test { app, http ->
-        app.get("/read-cookie-2") { ctx -> ctx.result(ctx.cookie("my-cookie")!!) }
-        val response = Unirest.get("${http.origin}/read-cookie-2").header(Header.COOKIE, "my-cookie=my-cookie-value").asString()
-        assertThat(response.body).isEqualTo("my-cookie-value")
-    }
-
-    @Test
-    fun `cookie-map returns empty when no cookies are set`() = TestUtil.test { app, http ->
-        app.get("/read-cookie-3") { ctx -> ctx.result(ctx.cookieMap().toString()) }
-        assertThat(http.getBody("/read-cookie-3")).isEqualTo("{}")
-    }
-
-    @Test
-    fun `cookie-map returns all cookies if cookies are set`() = TestUtil.test { app, http ->
-        app.get("/read-cookie-4") { ctx -> ctx.result(ctx.cookieMap().toString()) }
-        val response = Unirest.get("${http.origin}/read-cookie-4").header(Header.COOKIE, "k1=v1;k2=v2;k3=v3").asString()
-        assertThat(response.body).isEqualTo("{k1=v1, k2=v2, k3=v3}")
-    }
-
     /*
      * Path params
      */
diff --git a/javalin/src/test/java/io/javalin/TestResponse.kt b/javalin/src/test/java/io/javalin/TestResponse.kt
index 16994467..da9b4ed6 100644
--- a/javalin/src/test/java/io/javalin/TestResponse.kt
+++ b/javalin/src/test/java/io/javalin/TestResponse.kt
@@ -122,47 +122,6 @@ class TestResponse {
         assertThat(http.call(HttpMethod.GET, "/hello-abs").body).isEqualTo("Redirected")
     }
 
-    @Test
-    fun `setting a cookie works`() = TestUtil.test { app, http ->
-        app.get("/create-cookie") { ctx -> ctx.cookie("Test", "Tast") }
-        app.get("/get-cookie") { ctx -> ctx.result(ctx.cookie("Test")!!) }
-        assertThat(http.get("/create-cookie").headers.getFirst(Header.SET_COOKIE)).isEqualTo("Test=Tast; Path=/")
-        assertThat(http.getBody("/get-cookie")).isEqualTo("Tast")
-    }
-
-    @Test
-    fun `setting a Cookie object works`() = TestUtil.test { app, http ->
-        app.get("/create-cookie") { ctx -> ctx.cookie(Cookie("Hest", "Hast").apply { maxAge = 7 }) }
-        assertThat(http.get("/create-cookie").headers.getFirst(Header.SET_COOKIE)).contains("Hest=Hast")
-        assertThat(http.get("/create-cookie").headers.getFirst(Header.SET_COOKIE)).contains("Max-Age=7")
-    }
-
-    @Test
-    fun `can't set duplicate cookies when other cookies set`() = TestUtil.test { app, http ->
-        app.get("/create-cookies") {
-            it.cookie("Test-1", "1")
-            it.cookie("Test-2", "2")
-            it.cookie("Test-3", "3")
-            it.cookie("Test-3", "4")  // duplicate
-        }
-        val response = http.get("/create-cookies");
-        assertThat(response.headers[Header.SET_COOKIE]!!).contains("Test-1=1; Path=/")
-        assertThat(response.headers[Header.SET_COOKIE]!!).contains("Test-2=2; Path=/")
-        assertThat(response.headers[Header.SET_COOKIE]!!).contains("Test-3=4; Path=/")
-        assertThat(response.headers[Header.SET_COOKIE]!!.size).isEqualTo(3)
-    }
-
-    @Test
-    fun `can't set duplicate cookies when no other cookies set`() = TestUtil.test { app, http ->
-        app.get("/create-cookies") {
-            it.cookie("MyCookie", "A")
-            it.cookie("MyCookie", "B")  // duplicate
-        }
-        val response = http.get("/create-cookies");
-        assertThat(response.headers[Header.SET_COOKIE]!!).contains("MyCookie=B; Path=/")
-        assertThat(response.headers[Header.SET_COOKIE]!!.size).isEqualTo(1)
-    }
-
     // Fix for https://github.com/tipsy/javalin/issues/543
     @Test
     fun `reading the result string resets the stream`() = TestUtil.test { app, http ->