Skip to content

Commit

Permalink
GET Submissions: Do not merge into [action] (#464)
Browse files Browse the repository at this point in the history
Follow-up to [#461][]

While the changes made in #461 added coverage for `<form method="post"
action="...">` and `<form>` submissions, it didn't cover `<form
action="/?encoded=value">` submissions with search parameters encoded
into the `[action]` attribute.

Without Turbo or any other JavaScript, submitting a form with an
`[action]` value that contains query parameters requests that `[action]`
path without any key-value pairs (that is, with `?` as its the value of
`URL.search`):

```html
<form action="/src/tests/fixtures/form.html?sort=asc">
  <!-- clicking the button navigates to `/src/tests/fixtures/form?` -->
  <button>Submit</button>
</form>
```

This commit modifies the `FetchRequest` to always invoke the
module-private `mergeFormDataEntries()` function when constructing `GET`
requests, and modifies the `mergeFormDataEntries()` to completely
discard the `[action]` URL's search parameters before merging-in the
`FormData` key-value pairs.

[#461]: #461
  • Loading branch information
seanpdoyle authored Nov 22, 2021
1 parent b88bb24 commit aa61e6a
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
20 changes: 5 additions & 15 deletions src/http/fetch_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class FetchRequest {
this.method = method
this.headers = this.defaultHeaders
this.body = body
this.url = this.isIdempotent && this.entries.length ?
this.url = this.isIdempotent ?
mergeFormDataEntries(new URL(location.href), this.entries) :
location
this.target = target
Expand Down Expand Up @@ -152,25 +152,15 @@ export class FetchRequest {
}

function mergeFormDataEntries(url: URL, entries: [string, FormDataEntryValue][]): URL {
const currentSearchParams = new URLSearchParams(url.search)
deleteAll(url.searchParams)
const searchParams = new URLSearchParams

for (const [ name, value ] of entries) {
if (value instanceof File) continue

if (currentSearchParams.has(name)) {
currentSearchParams.delete(name)
url.searchParams.set(name, value)
} else {
url.searchParams.append(name, value)
}
searchParams.append(name, value)
}

return url
}
url.search = searchParams.toString()

function deleteAll(searchParams: URLSearchParams) {
for (const name of searchParams.keys()) {
searchParams.delete(name)
}
return url
}
7 changes: 5 additions & 2 deletions src/tests/fixtures/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ <h1>Form</h1>
<form>
<button id="form-action-none-q-a" name="q" value="a">Submit ?q=a to form:not([action])</button>
</form>
<form action="/src/tests/fixtures/form.html?sort=asc">
<button id="form-action-self-submit">Submit form[action]</button>
<form action="/src/tests/fixtures/form.html">
<button id="form-action-self-sort" name="sort" value="asc">Submit ?sort=asc to form[action]</button>
<button id="form-action-self-q-b" name="q" value="b">Submit ?q=b to form[action]</button>
</form>
<form action="/src/tests/fixtures/form.html?q=c">
<button id="form-action-self-submit">Submit form[action]</button>
</form>
<form action="/__turbo/redirect?path=%2Fsrc%2Ftests%2Ffixtures%2Fform.html%3Fsort%3Dasc" method="post">
<button id="form-action-post-redirect-self-q-b" name="q" value="b">POST q=b to form[action]</button>
</form>
Expand Down
12 changes: 10 additions & 2 deletions src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export class FormSubmissionTests extends TurboDriveTestCase {
}

async "test standard GET form submission does not incorporate the current page's URLSearchParams values into the submission"() {
await this.clickSelector("#form-action-self-submit")
await this.clickSelector("#form-action-self-sort")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
Expand All @@ -150,7 +150,7 @@ export class FormSubmissionTests extends TurboDriveTestCase {
}

async "test standard GET form submission does not merge values into the [action] attribute"() {
await this.clickSelector("#form-action-self-submit")
await this.clickSelector("#form-action-self-sort")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
Expand All @@ -163,6 +163,14 @@ export class FormSubmissionTests extends TurboDriveTestCase {
this.assert.equal(await this.search, "?q=b", "navigates without omitted keys")
}

async "test standard GET form submission omits the [action] value's URLSearchParams from the submission"() {
await this.clickSelector("#form-action-self-submit")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.equal(await this.search, "")
}

async "test standard GET form submission toggles submitter [disabled] attribute"() {
await this.clickSelector("#standard-get-form-submit")

Expand Down

0 comments on commit aa61e6a

Please sign in to comment.