Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.*

Expand Down Expand Up @@ -38,7 +38,7 @@ fun Route.aboutPage(

populateCommunityCalloutModel(serverConfig, model, accountsRepository, projectAccessRepository)

call.respond(MustacheContent("about.mustache", model))
call.respondPage("about.mustache", model)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -48,7 +47,7 @@ fun Route.homePage(

populateCommunityCalloutModel(serverConfig, model, accountsRepository, projectAccessRepository)

call.respond(MustacheContent("home.mustache", model))
call.respondPage("home.mustache", model)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
}
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,20 @@ suspend fun ApplicationCall.msg(data: MutableMap<String, Any>, key: String, vara
val message = msg(key, *args)
val msgData = data["msg"] as? MutableMap<String, String> ?: 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<String, String>() }
as SortedMap<String, String>
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<String, Any> = emptyMap()): MutableMap<String, Any> {
val locale = getLocale()
val bundle = messagesBundle(locale)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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<String, Any>,
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")
Original file line number Diff line number Diff line change
@@ -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"])
}
}
Loading