Restore correct Accept header for form submissions

Closes #141. Follows-up #52. --- When creating the headers for a `FetchRequest`, provide the default headers to the delegate by adding an argument to the `additionalHeadersForRequest` signature so that delegates can incorporate the current values if they choose to. They're passed as a copy to prevent delegates from destructively acting upon them, with the intention that delegates merge values _into_ them. Testing --- Add a guard middleware to the test server to reject _all_ requests that don't specify an [Accept][] header containing `"text/html, application/xhtml+xml"`. Next, guard Stream requests with a similar check for `"text/vnd.turbo-stream.html"`. Since those endpoints are behind the guard middleware, they'll also require `"text/html, application/xhtml+xml"` as well. Finally, replace the Stream functional test's explicit call to `fetch` with a `<form>` element submitting with the same content. [Accept]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
hotwired · Jan 29, 2021 · ca350d9 · ca350d9
1 parent 4c439b9
commit ca350d9
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 26 deletions.
diff --git a/src/core/drive/form_submission.ts b/src/core/drive/form_submission.ts
@@ -104,16 +104,16 @@ export class FormSubmission {
 
   // Fetch request delegate
 
-  additionalHeadersForRequest(request: FetchRequest) {
-    const headers: FetchRequestHeaders = {}
+  additionalHeadersForRequest(request: FetchRequest, headers: { [header: string]: string }) {
+    const additionalHeaders: FetchRequestHeaders = {}
     if (!request.isIdempotent) {
       const token = getCookieValue(getMetaContent("csrf-param")) || getMetaContent("csrf-token")
       if (token) {
-        headers["X-CSRF-Token"] = token
+        additionalHeaders["X-CSRF-Token"] = token
       }
-      headers["Accept"] = StreamMessage.contentType
+      additionalHeaders["Accept"] = [ StreamMessage.contentType, headers.Accept ].join(", ")
     }
-    return headers
+    return additionalHeaders
   }
 
   requestStarted(request: FetchRequest) {

diff --git a/src/http/fetch_request.ts b/src/http/fetch_request.ts
@@ -2,7 +2,7 @@ import { FetchResponse } from "./fetch_response"
 import { dispatch } from "../util"
 
 export interface FetchRequestDelegate {
-  additionalHeadersForRequest?(request: FetchRequest): { [header: string]: string }
+  additionalHeadersForRequest?(request: FetchRequest, headers: { [header: string]: string }): { [header: string]: string }
   requestStarted(request: FetchRequest): void
   requestPreventedHandlingResponse(request: FetchRequest, response: FetchResponse): void
   requestSucceededWithResponse(request: FetchRequest, response: FetchResponse): void
@@ -117,15 +117,15 @@ export class FetchRequest {
   }
 
   get headers() {
-    return {
-      "Accept": "text/html, application/xhtml+xml",
-      ...this.additionalHeaders
-    }
+    const defaultHeaders = { "Accept": "text/html, application/xhtml+xml" }
+    const additionalHeaders = this.additionalHeadersWithDefaults(defaultHeaders)
+
+    return { ...defaultHeaders, ...additionalHeaders }
   }
 
-  get additionalHeaders() {
+  additionalHeadersWithDefaults(defaults: { [header: string]: string }) {
     if (typeof this.delegate.additionalHeadersForRequest == "function") {
-      return this.delegate.additionalHeadersForRequest(this)
+      return this.delegate.additionalHeadersForRequest(this, { ...defaults })
     } else {
       return {}
     }

diff --git a/src/tests/fixtures/stream.html b/src/tests/fixtures/stream.html
@@ -7,6 +7,11 @@
     <script>Turbo.connectStreamSource(new EventSource("/__turbo/messages"))</script>
   </head>
   <body>
+    <form id="create" method="post" action="/__turbo/messages">
+      <input type="hidden" name="content" value="Hello world!">
+      <input type="hidden" name="type" value="stream">
+      <button type="submit">Create</button>
+    </form>
     <div id="messages">
       <div class="message">First</div>
     </div>

diff --git a/src/tests/functional/stream_tests.ts b/src/tests/functional/stream_tests.ts
@@ -12,22 +12,12 @@ export class StreamTests extends FunctionalTestCase {
     element = await this.querySelector(selector)
     this.assert.equal(await element.getVisibleText(), "First")
 
-    await this.createMessage("Hello world!")
+    await this.clickSelector("#create [type=submit]")
     await this.nextBeat
 
     element = await this.querySelector(selector)
     this.assert.equal(await element.getVisibleText(), "Hello world!")
   }
-
-  async createMessage(content: string) {
-    return this.post("/__turbo/messages", { content })
-  }
-
-  async post(path: string, params: any = {}) {
-    await this.evaluate((path, method, params) => {
-      fetch(location.origin + path, { method, body: new URLSearchParams(params) })
-    }, path, "POST", params)
-  }
 }
 
 StreamTests.registerSuite()
diff --git a/src/tests/server.ts b/src/tests/server.ts
@@ -1,4 +1,4 @@
-import { Response, Router } from "express"
+import { Request, Response, Router } from "express"
 import multer from "multer"
 import path from "path"
 import url from "url"
@@ -8,6 +8,14 @@ const streamResponses: Set<Response> = new Set
 
 router.use(multer().none())
 
+router.use((request, response, next) => {
+  if (request.accepts(["text/html", "application/xhtml+xml"])) {
+    next()
+  } else {
+    response.sendStatus(422)
+  }
+})
+
 router.post("/redirect", (request, response) => {
   const { path, ...query } = request.body
   const pathname = path ?? "/src/tests/fixtures/one.html"
@@ -39,7 +47,7 @@ router.post("/messages", (request, response) => {
   const { content, status, type } = request.body
   if (typeof content == "string") {
     receiveMessage(content)
-    if (type == "stream") {
+    if (type == "stream" && acceptsStreams(request)) {
       response.type("text/vnd.turbo-stream.html; charset=utf-8")
       response.send(renderMessage(content))
     } else {
@@ -55,7 +63,7 @@ router.put("/messages/:id", (request, response) => {
   const { id } = request.params
   if (typeof content == "string") {
     receiveMessage(content)
-    if (type == "stream") {
+    if (type == "stream" &&  acceptsStreams(request)) {
       response.type("text/vnd.turbo-stream.html; charset=utf-8")
       response.send(renderMessage(id + ": " + content))
     } else {
@@ -99,6 +107,10 @@ function renderMessage(content: string) {
   `
 }
 
+function acceptsStreams(request: Request): boolean {
+  return !!request.accepts("text/vnd.turbo-stream.html")
+}
+
 function renderSSEData(data: any) {
   return `${data}`.split("\n").map(line => "data:" + line).join("\n") + "\n\n"
 }