From fcbb34006f1996a4cae412b419c9e9fc260528d9 Mon Sep 17 00:00:00 2001 From: Mariia Skripchenko <61115099+marychatte@users.noreply.github.com> Date: Mon, 17 Jun 2024 16:06:39 +0200 Subject: [PATCH] KTOR-6655 Fix missing charset for static js, css and svg resources (#4058) (cherry picked from commit c892bc0f4fe1f1bc8bacc66c1d6856cdf6baf04b) --- .../src/io/ktor/http/FileContentType.kt | 25 +++- ktor-http/common/src/io/ktor/http/Mimes.kt | 1 + .../io/ktor/server/webjars/WebjarsTest.kt | 109 +++++++++--------- .../jvm/test-resources/public/types/file.css | 0 .../jvm/test-resources/public/types/file.js | 0 .../jvm/test-resources/public/types/file.svg | 0 .../jvm/test-resources/public/types/file.xml | 0 .../ktor/server/plugins/StaticContentTest.kt | 21 ++++ 8 files changed, 99 insertions(+), 57 deletions(-) create mode 100644 ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.css create mode 100644 ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.js create mode 100644 ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.svg create mode 100644 ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.xml diff --git a/ktor-http/common/src/io/ktor/http/FileContentType.kt b/ktor-http/common/src/io/ktor/http/FileContentType.kt index 1c921d6ab6a..9bb1327ed22 100644 --- a/ktor-http/common/src/io/ktor/http/FileContentType.kt +++ b/ktor-http/common/src/io/ktor/http/FileContentType.kt @@ -65,11 +65,34 @@ private val extensionsByContentType: Map> by lazy { internal fun List.selectDefault(): ContentType { val contentType = firstOrNull() ?: ContentType.Application.OctetStream return when { - contentType.contentType == "text" && contentType.charset() == null -> contentType.withCharset(Charsets.UTF_8) + contentType.match(ContentType.Text.Any) -> contentType.withCharsetUTF8IfNeeded() + contentType.match(ContentType.Image.SVG) -> contentType.withCharsetUTF8IfNeeded() + contentType.matchApplicationTypeWithCharset() -> contentType.withCharsetUTF8IfNeeded() else -> contentType } } +private fun ContentType.matchApplicationTypeWithCharset(): Boolean { + if (!match(ContentType.Application.Any)) return false + + return when { + match(ContentType.Application.Atom) || + match(ContentType.Application.JavaScript) || + match(ContentType.Application.Rss) || + match(ContentType.Application.Xml) || + match(ContentType.Application.Xml_Dtd) + -> true + + else -> false + } +} + +private fun ContentType.withCharsetUTF8IfNeeded(): ContentType { + if (charset() != null) return this + + return withCharset(Charsets.UTF_8) +} + internal fun Sequence>.groupByPairs() = groupBy { it.first } .mapValues { e -> e.value.map { it.second } } diff --git a/ktor-http/common/src/io/ktor/http/Mimes.kt b/ktor-http/common/src/io/ktor/http/Mimes.kt index 0e243d483f1..a5fd21ffd23 100644 --- a/ktor-http/common/src/io/ktor/http/Mimes.kt +++ b/ktor-http/common/src/io/ktor/http/Mimes.kt @@ -428,6 +428,7 @@ private val rawMimes: String .jpgv,video/jpeg .jpm,video/jpm .jps,image/x-jps +.js,text/javascript .js,application/javascript .json,application/json .jut,image/jutvision diff --git a/ktor-server/ktor-server-plugins/ktor-server-webjars/jvm/test/io/ktor/server/webjars/WebjarsTest.kt b/ktor-server/ktor-server-plugins/ktor-server-webjars/jvm/test/io/ktor/server/webjars/WebjarsTest.kt index 362cc7d67a7..aa9a36f6cfb 100644 --- a/ktor-server/ktor-server-plugins/ktor-server-webjars/jvm/test/io/ktor/server/webjars/WebjarsTest.kt +++ b/ktor-server/ktor-server-plugins/ktor-server-webjars/jvm/test/io/ktor/server/webjars/WebjarsTest.kt @@ -12,135 +12,132 @@ import io.ktor.server.plugins.conditionalheaders.* import io.ktor.server.response.* import io.ktor.server.routing.* import io.ktor.server.testing.* -import io.ktor.util.* import kotlin.test.* -@Suppress("DEPRECATION") class WebjarsTest { @Test fun resourceNotFound() { - withTestApplication { - application.install(Webjars) - handleRequest(HttpMethod.Get, "/webjars/foo.js").let { call -> + testApplication { + install(Webjars) + client.get("/webjars/foo.js").let { response -> // Should be handled by some other routing - assertEquals(HttpStatusCode.NotFound, call.response.status()) + assertEquals(HttpStatusCode.NotFound, response.status) } } } @Test fun pathLike() { - withTestApplication { - application.install(Webjars) - application.routing { + testApplication { + install(Webjars) + routing { get("/webjars-something/jquery") { call.respondText { "Something Else" } } } - handleRequest(HttpMethod.Get, "/webjars-something/jquery").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("Something Else", call.response.content) + client.get("/webjars-something/jquery").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals("Something Else", response.bodyAsText()) } } } @Test fun nestedPath() { - withTestApplication { - application.install(Webjars) { + testApplication { + install(Webjars) { path = "/assets/webjars" } - handleRequest(HttpMethod.Get, "/assets/webjars/jquery/jquery.js").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) + client.get("/assets/webjars/jquery/jquery.js").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } @Test fun rootPath() { - withTestApplication { - application.install(Webjars) { + testApplication { + install(Webjars) { path = "/" } - handleRequest(HttpMethod.Get, "/jquery/jquery.js").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) + client.get("/jquery/jquery.js").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } @Test fun rootPath2() { - withTestApplication { - application.install(Webjars) { + testApplication { + install(Webjars) { path = "/" } - application.routing { + routing { get("/") { call.respondText("Hello, World") } } - handleRequest(HttpMethod.Get, "/").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("Hello, World", call.response.content) + client.get("/").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals("Hello, World", response.bodyAsText()) } - handleRequest(HttpMethod.Get, "/jquery/jquery.js").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) + client.get("/jquery/jquery.js").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } @Test fun versionAgnostic() { - withTestApplication { - application.install(Webjars) + testApplication { + install(Webjars) - handleRequest(HttpMethod.Get, "/webjars/jquery/jquery.js").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) + client.get("/webjars/jquery/jquery.js").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } @Test fun withGetParameters() { - withTestApplication { - application.install(Webjars) + testApplication { + install(Webjars) - handleRequest(HttpMethod.Get, "/webjars/jquery/jquery.js?param1=value1").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) + client.get("/webjars/jquery/jquery.js?param1=value1").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } @Test fun withSpecificVersion() { - withTestApplication { - application.install(Webjars) + testApplication { + install(Webjars) - handleRequest(HttpMethod.Get, "/webjars/jquery/3.6.4/jquery.js").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) + client.get("/webjars/jquery/3.6.4/jquery.js").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } @Test - fun withConditionalHeaders() { - withTestApplication { - application.install(Webjars) - application.install(ConditionalHeaders) - handleRequest(HttpMethod.Get, "/webjars/jquery/3.6.4/jquery.js").let { call -> - assertEquals(HttpStatusCode.OK, call.response.status()) - assertEquals("application/javascript", call.response.headers["Content-Type"]) - assertNotNull(call.response.headers["Last-Modified"]) + fun withConditionalAndCachingHeaders() { + testApplication { + install(Webjars) + install(ConditionalHeaders) + client.get("/webjars/jquery/3.6.4/jquery.js").let { response -> + assertEquals(HttpStatusCode.OK, response.status) + assertEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) + assertNotNull(response.headers["Last-Modified"]) } } } - @OptIn(InternalAPI::class) @Test fun callHandledBeforeWebjars() { val alwaysRespondHello = object : Hook { @@ -161,7 +158,7 @@ class WebjarsTest { val response = client.get("/webjars/jquery/3.3.1/jquery.js") assertEquals(HttpStatusCode.OK, response.status) assertEquals("Hello", response.bodyAsText()) - assertNotEquals("application/javascript", response.headers["Content-Type"]) + assertNotEquals(ContentType.Text.JavaScript, response.contentType()?.withoutParameters()) } } } diff --git a/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.css b/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.css new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.js b/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.svg b/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.svg new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.xml b/ktor-server/ktor-server-tests/jvm/test-resources/public/types/file.xml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/StaticContentTest.kt b/ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/StaticContentTest.kt index f47625e3888..99ff928bdfe 100644 --- a/ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/StaticContentTest.kt +++ b/ktor-server/ktor-server-tests/jvm/test/io/ktor/server/plugins/StaticContentTest.kt @@ -973,6 +973,27 @@ class StaticContentTest { assertEquals("br", result.headers[HttpHeaders.ContentEncoding].orEmpty()) } } + + @Test + fun testCharset() = testApplication { + val fileName = "file" + val extensions = mapOf( + "js" to ContentType.Text.JavaScript, + "css" to ContentType.Text.CSS, + "svg" to ContentType.Image.SVG, + "xml" to ContentType.Application.Xml, + ) + + routing { + staticResources("/", "public/types") + } + + extensions.forEach { (extension, contentType) -> + client.get("/$fileName.$extension").apply { + assertEquals(contentType.withCharset(Charsets.UTF_8), contentType()) + } + } + } } private fun String.replaceSeparators() = replace("/", File.separator)