diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AboutPage.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AboutPage.kt index a1f4cc625..463045660 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AboutPage.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AboutPage.kt @@ -6,10 +6,10 @@ import com.darkrockstudios.apps.hammer.admin.AdminServerConfig import com.darkrockstudios.apps.hammer.admin.ConfigRepository import com.darkrockstudios.apps.hammer.frontend.utils.metaDescription import com.darkrockstudios.apps.hammer.frontend.utils.msg +import com.darkrockstudios.apps.hammer.frontend.utils.respondPage import com.darkrockstudios.apps.hammer.project.access.ProjectAccessRepository import com.darkrockstudios.apps.hammer.utilities.MarkdownService import io.ktor.http.* -import io.ktor.server.mustache.* import io.ktor.server.response.* import io.ktor.server.routing.* @@ -38,7 +38,7 @@ fun Route.aboutPage( populateCommunityCalloutModel(serverConfig, model, accountsRepository, projectAccessRepository) - call.respond(MustacheContent("about.mustache", model)) + call.respondPage("about.mustache", model) } } } diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AuthorPage.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AuthorPage.kt index a12531404..e0083a497 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AuthorPage.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/AuthorPage.kt @@ -7,6 +7,7 @@ import com.darkrockstudios.apps.hammer.frontend.utils.authorProfileJsonLd import com.darkrockstudios.apps.hammer.frontend.utils.canonicalUrl import com.darkrockstudios.apps.hammer.frontend.utils.metaDescription import com.darkrockstudios.apps.hammer.frontend.utils.resolveByPenName +import com.darkrockstudios.apps.hammer.frontend.utils.respondPage import com.darkrockstudios.apps.hammer.project.access.ProjectAccessRepository import com.darkrockstudios.apps.hammer.utilities.MarkdownService import io.ktor.http.* @@ -97,7 +98,9 @@ fun Route.authorPage( description = metaDescription(account.bio), ) } - call.respond(MustacheContent("author.mustache", model)) + // `community_member` is an explicit input: it gates the X-Robots-Tag header, which + // never reaches the model. + call.respondPage("author.mustache", model, account.community_member) } } } diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/HomePage.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/HomePage.kt index 12237cbe4..0a09b68f7 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/HomePage.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/HomePage.kt @@ -7,10 +7,9 @@ import com.darkrockstudios.apps.hammer.admin.ConfigRepository import com.darkrockstudios.apps.hammer.admin.WhiteListRepository import com.darkrockstudios.apps.hammer.frontend.utils.canonicalUrl import com.darkrockstudios.apps.hammer.frontend.utils.msg +import com.darkrockstudios.apps.hammer.frontend.utils.respondPage import com.darkrockstudios.apps.hammer.frontend.utils.webSiteJsonLd import com.darkrockstudios.apps.hammer.project.access.ProjectAccessRepository -import io.ktor.server.mustache.* -import io.ktor.server.response.* import io.ktor.server.routing.* fun Route.homePage( @@ -48,7 +47,7 @@ fun Route.homePage( populateCommunityCalloutModel(serverConfig, model, accountsRepository, projectAccessRepository) - call.respond(MustacheContent("home.mustache", model)) + call.respondPage("home.mustache", model) } } } diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/PrivacyPolicyPage.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/PrivacyPolicyPage.kt index 5cfa7cb38..c61766c98 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/PrivacyPolicyPage.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/PrivacyPolicyPage.kt @@ -1,9 +1,9 @@ package com.darkrockstudios.apps.hammer.frontend import com.darkrockstudios.apps.hammer.account.PrivacyPolicyRepository +import com.darkrockstudios.apps.hammer.frontend.utils.respondPage import com.darkrockstudios.apps.hammer.utilities.MarkdownService import io.ktor.http.* -import io.ktor.server.mustache.* import io.ktor.server.response.* import io.ktor.server.routing.* import org.koin.ktor.ext.inject @@ -23,7 +23,7 @@ fun Route.privacyPolicyPage() { model["page_stylesheet"] = "/assets/css/about.css" model["privacyHtml"] = markdownService.markdownToSafeHtml(markdown) - call.respond(MustacheContent("privacy.mustache", model)) + call.respondPage("privacy.mustache", model) } } } diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/TermsOfServicePage.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/TermsOfServicePage.kt index 1a493fba2..a96a9a890 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/TermsOfServicePage.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/TermsOfServicePage.kt @@ -1,8 +1,8 @@ package com.darkrockstudios.apps.hammer.frontend import com.darkrockstudios.apps.hammer.account.TermsOfServiceRepository +import com.darkrockstudios.apps.hammer.frontend.utils.respondPage import io.ktor.http.* -import io.ktor.server.mustache.* import io.ktor.server.response.* import io.ktor.server.routing.* import org.koin.ktor.ext.inject @@ -20,7 +20,7 @@ fun Route.termsOfServicePage() { val model = call.withDefaults() model["termsText"] = challenge.text - call.respond(MustacheContent("terms.mustache", model)) + call.respondPage("terms.mustache", model) } } } diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/Localization.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/Localization.kt index 09af68071..5c56e35fd 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/Localization.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/Localization.kt @@ -61,8 +61,20 @@ suspend fun ApplicationCall.msg(data: MutableMap, key: String, vara val message = msg(key, *args) val msgData = data["msg"] as? MutableMap ?: mutableMapOf() msgData[key] = message + + // Thar be dragons: pageETag skips the bulk `msg` bundle because the bundle is determined by + // `locale` and `version`. A message formatted here is not — it carries request or config data + // the bundle never saw. Mirroring it into a hashed key keeps such a value inside the validator; + // without this, changing the data behind a formatted message serves everyone a stale page. + @Suppress("UNCHECKED_CAST") + val formatted = data.getOrPut(FORMATTED_MSG_KEY) { sortedMapOf() } + as SortedMap + formatted[key] = message } +/** Model key holding the runtime-formatted messages, mirrored out of `msg` so [pageETag] sees them. */ +const val FORMATTED_MSG_KEY = "msgFormatted" + suspend fun ApplicationCall.withMessages(data: Map = emptyMap()): MutableMap { val locale = getLocale() val bundle = messagesBundle(locale) diff --git a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/PageETag.kt b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/PageETag.kt index 11579a281..fbe009b17 100644 --- a/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/PageETag.kt +++ b/server/src/main/kotlin/com/darkrockstudios/apps/hammer/frontend/utils/PageETag.kt @@ -2,9 +2,12 @@ package com.darkrockstudios.apps.hammer.frontend.utils import com.darkrockstudios.apps.hammer.utilities.sha256Hex import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode import io.ktor.server.application.ApplicationCall +import io.ktor.server.mustache.MustacheContent import io.ktor.server.request.header import io.ktor.server.response.header +import io.ktor.server.response.respond /** * A validator for a server-rendered page: the SHA-256 of its Mustache model plus any [extra] inputs @@ -13,7 +16,8 @@ import io.ktor.server.response.header * Hashing the model itself — rather than an enumerated list of inputs — means a field added to a * page automatically joins its validator, instead of quietly serving a stale page to everyone * holding the old one. `msg` and `locales` are skipped: they are bulky and fully determined by the - * `locale` and `version` entries, which are hashed. + * `locale` and `version` entries, which are hashed. A message formatted at request time is the one + * exception, so [FORMATTED_MSG_KEY] mirrors those back into the hash. * * Weak, because Compression may re-encode the body and a strong validator promises byte equality. */ @@ -48,4 +52,29 @@ fun ApplicationCall.applyRevalidationHeaders(etag: String) { response.header(HttpHeaders.Vary, "Cookie, Accept-Language") } +/** + * Renders [template] with [model], or answers 304 when the caller already holds that exact page. + * + * Hashing happens here, once the model is complete, so every value that shapes the page is in the + * validator by construction. This saves the Mustache render and the response body — not the work + * that built the model. A page with a cheap fingerprint available *before* its expensive step + * should hash early instead and skip that step too, the way the public story reader does. + * + * Anything that shapes the response without living in the model — a header such as `X-Robots-Tag` — + * must be passed as [extra], or two different responses will share one validator. + */ +suspend fun ApplicationCall.respondPage( + template: String, + model: Map, + vararg extra: Any?, +) { + val etag = pageETag(model, *extra) + applyRevalidationHeaders(etag) + if (matchesETag(etag)) { + respond(HttpStatusCode.NotModified) + } else { + respond(MustacheContent(template, model)) + } +} + private val UNHASHED_MODEL_KEYS = setOf("msg", "locales") diff --git a/server/src/test/kotlin/com/darkrockstudios/apps/hammer/e2e/PageValidatorTest.kt b/server/src/test/kotlin/com/darkrockstudios/apps/hammer/e2e/PageValidatorTest.kt new file mode 100644 index 000000000..00c9dbd71 --- /dev/null +++ b/server/src/test/kotlin/com/darkrockstudios/apps/hammer/e2e/PageValidatorTest.kt @@ -0,0 +1,188 @@ +package com.darkrockstudios.apps.hammer.e2e + +import com.darkrockstudios.apps.hammer.e2e.util.E2eTestData +import com.darkrockstudios.apps.hammer.e2e.util.EndToEndTest +import com.darkrockstudios.apps.hammer.e2e.util.TestAccount +import io.ktor.client.request.get +import io.ktor.client.request.header +import io.ktor.client.statement.bodyAsText +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * HTTP validators on the server-rendered Mustache pages: that each carries one, that a caller + * holding it is answered 304, and that every input which changes the page also changes it. + */ +class PageValidatorTest : EndToEndTest() { + + private val userId = 1L + private val penName = "JaneAuthor" + + private fun setConfig(key: String, value: String) { + database().serverDatabase.serverConfigQueries.upsertConfig(key, value) + } + + /** Any account at all; without one the server is in setup mode and redirects every page. */ + private fun seedAccount() = runBlocking { + E2eTestData.createAccount(TestAccount(email = "jane@test.com", password = "password123!@#"), database()) + } + + private fun seedAuthor(bio: String?, communityMember: Boolean = true) { + seedAccount() + with(database().serverDatabase.accountQueries) { + updatePenName(pen_name = penName, id = userId) + updateBio(bio = bio, id = userId) + updateCommunityMember(community_member = communityMember, id = userId) + } + } + + private suspend fun etagOf(path: String): String { + val response = client().get(route(path)) + assertEquals(HttpStatusCode.OK, response.status, "$path should render") + return assertNotNull(response.headers[HttpHeaders.ETag], "$path should carry a validator") + } + + @Test + fun `the home page is served with a revalidation validator`(): Unit = runBlocking { + doStartServer() + seedAccount() + + val response = client().get(route("")) + + assertEquals(HttpStatusCode.OK, response.status) + val etag = assertNotNull(response.headers[HttpHeaders.ETag], "the page should carry a validator") + assertTrue(etag.startsWith("W/\""), "the validator should be weak, was $etag") + assertEquals("private, no-cache", response.headers[HttpHeaders.CacheControl]) + assertEquals("Cookie, Accept-Language", response.headers[HttpHeaders.Vary]) + } + + @Test + fun `a visitor holding the home page validator is answered 304`(): Unit = runBlocking { + doStartServer() + seedAccount() + + val etag = etagOf("") + val second = client().get(route("")) { header(HttpHeaders.IfNoneMatch, etag) } + + assertEquals(HttpStatusCode.NotModified, second.status) + assertTrue(second.bodyAsText().isEmpty(), "a 304 carries no body") + } + + @Test + fun `changing the server message changes the home page validator`(): Unit = runBlocking { + doStartServer() + seedAccount() + + val before = etagOf("") + setConfig("server_message", "The instance is moving to a new host on Friday.") + + val response = client().get(route("")) { header(HttpHeaders.IfNoneMatch, before) } + + assertEquals(HttpStatusCode.OK, response.status, "a changed server message must not answer 304") + assertNotEquals(before, response.headers[HttpHeaders.ETag]) + assertTrue(response.bodyAsText().contains("moving to a new host")) + } + + /** + * The contact email reaches the page only through a message formatted at request time, which + * lands in the bulky `msg` bundle that the validator deliberately skips. Without the mirror + * that puts formatted messages back in the hash, an admin changing the address would leave the + * validator identical and every browser holding it would keep showing the old address. + */ + @Test + fun `changing an address that only appears in a formatted message changes the validator`(): Unit = runBlocking { + doStartServer() + seedAccount() + setConfig("contact_email", "old-admin@test.com") + + val first = client().get(route("")) + val before = assertNotNull(first.headers[HttpHeaders.ETag]) + assertTrue(first.bodyAsText().contains("old-admin@test.com"), "the formatted message should render") + + setConfig("contact_email", "new-admin@test.com") + val response = client().get(route("")) { header(HttpHeaders.IfNoneMatch, before) } + + assertEquals(HttpStatusCode.OK, response.status, "a changed contact address must not answer 304") + assertNotEquals(before, response.headers[HttpHeaders.ETag]) + assertTrue(response.bodyAsText().contains("new-admin@test.com")) + } + + @Test + fun `a reader holding the about page validator is answered 304`(): Unit = runBlocking { + doStartServer() + seedAccount() + setConfig("about_server", "This instance is run by a writing group.") + + val etag = etagOf("about") + val second = client().get(route("about")) { header(HttpHeaders.IfNoneMatch, etag) } + + assertEquals(HttpStatusCode.NotModified, second.status) + } + + @Test + fun `editing the about text changes the about page validator`(): Unit = runBlocking { + doStartServer() + seedAccount() + setConfig("about_server", "This instance is run by a writing group.") + + val before = etagOf("about") + setConfig("about_server", "This instance is now invite only.") + + val response = client().get(route("about")) { header(HttpHeaders.IfNoneMatch, before) } + + assertEquals(HttpStatusCode.OK, response.status, "edited about text must not answer 304") + assertNotEquals(before, response.headers[HttpHeaders.ETag]) + assertTrue(response.bodyAsText().contains("now invite only")) + } + + @Test + fun `a reader holding the author page validator is answered 304`(): Unit = runBlocking { + doStartServer() + seedAuthor(bio = "Writes slowly, revises forever.") + + val etag = etagOf("a/$penName") + val second = client().get(route("a/$penName")) { header(HttpHeaders.IfNoneMatch, etag) } + + assertEquals(HttpStatusCode.NotModified, second.status) + } + + @Test + fun `editing an author bio changes the author page validator`(): Unit = runBlocking { + doStartServer() + seedAuthor(bio = "Writes slowly, revises forever.") + + val before = etagOf("a/$penName") + database().serverDatabase.accountQueries.updateBio(bio = "Now writing science fiction.", id = userId) + + val response = client().get(route("a/$penName")) { header(HttpHeaders.IfNoneMatch, before) } + + assertEquals(HttpStatusCode.OK, response.status, "an edited bio must not answer 304") + assertNotEquals(before, response.headers[HttpHeaders.ETag]) + assertTrue(response.bodyAsText().contains("Now writing science fiction.")) + } + + /** + * `community_member` gates the X-Robots-Tag header, which never reaches the model. It is passed + * to the validator explicitly, so two responses differing only in that header can't share one. + */ + @Test + fun `leaving the community changes the author page validator`(): Unit = runBlocking { + doStartServer() + seedAuthor(bio = "Writes slowly, revises forever.", communityMember = true) + + val before = etagOf("a/$penName") + database().serverDatabase.accountQueries.updateCommunityMember(community_member = false, id = userId) + + val response = client().get(route("a/$penName")) { header(HttpHeaders.IfNoneMatch, before) } + + assertEquals(HttpStatusCode.OK, response.status, "a de-indexed author page must not answer 304") + assertNotEquals(before, response.headers[HttpHeaders.ETag]) + assertEquals("noindex, nofollow", response.headers["X-Robots-Tag"]) + } +}