Skip to content

Commit

Permalink
GET form submissions to the current path
Browse files Browse the repository at this point in the history
Closes #107

The [HTML specification][] outlines the algorithm for handling form
submissions, and describes the steps involved in handling a `method=get`
request with `application/x-www-form-urlencoded` encoding.

However, it doesn't explicitly mention how to handle a `<form
method="get">` submission when the `action` attribute is omitted and the
current URL includes a query parameter that would be included within the
`<form>` element fields' values.

Determining the test cases to reproduce browser default behavior
involved was some trial and error troubleshooting in real browsers with
default (JavaScript-less) `<form>` submission handling.

The algorithm appears to be as follows:

* If there are no query parameters, append all field values as query
   parameters

* If there are existing query parameters whose keys exist as `<form>`
  fields, override them based on the form field's value

  * If there are existing query parameters whose keys exist as
    _multiple_ `<form>` fields, override the first occurrence, and then
    append subsequent values

[HTML specification]: https://html.spec.whatwg.org/#form-submission-algorithm
  • Loading branch information
seanpdoyle committed Jan 28, 2021
1 parent 14d3cdd commit 9efa22d
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 7 deletions.
11 changes: 10 additions & 1 deletion src/http/fetch_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,17 @@ export class FetchRequest {
}

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

for (const [ name, value ] of entries) {
url.searchParams.append(name, value.toString())
if (value instanceof File) continue

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

return url
Expand Down
16 changes: 16 additions & 0 deletions src/tests/fixtures/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@
<input type="hidden" name="greeting" value="Hello from a form">
<input type="submit">
</form>
<form action="/src/tests/fixtures/form.html?query=1" method="get" class="conflicting-values">
<input type="hidden" name="query" value="2">
<input type="submit">
</form>
</div>
<hr>
<div id="no-action">
<form class="single">
<input type="hidden" name="query" value="1">
<input type="submit">
</form>
<form class="multiple">
<input type="hidden" name="query" value="1">
<input type="hidden" name="query" value="2">
<input type="submit">
</form>
</div>
<hr>
<div id="reject">
Expand Down
57 changes: 51 additions & 6 deletions src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class FormSubmissionTests extends TurboDriveTestCase {
this.assert.ok(await this.formSubmitted)
this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.equal(await this.visitAction, "advance")
this.assert.equal((await this.searchParams).get("greeting"), "Hello from a redirect")
this.assert.equal(await this.getSearchParam("greeting"), "Hello from a redirect")
}

async "test standard GET form submission"() {
Expand All @@ -25,7 +25,16 @@ export class FormSubmissionTests extends TurboDriveTestCase {
this.assert.notOk(await this.formSubmitted)
this.assert.equal(await this.pathname, "/src/tests/fixtures/one.html")
this.assert.equal(await this.visitAction, "advance")
this.assert.equal((await this.searchParams).get("greeting"), "Hello from a form")
this.assert.equal(await this.getSearchParam("greeting"), "Hello from a form")
}

async "test standard GET form submission appending keys"() {
await this.goToLocation("/src/tests/fixtures/form.html?query=1")
await this.clickSelector("#standard form.conflicting-values input[type=submit]")
await this.nextBody

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

async "test standard form submission with empty created response"() {
Expand All @@ -52,26 +61,62 @@ export class FormSubmissionTests extends TurboDriveTestCase {
await this.clickSelector("#standard form[method=post][enctype] input[type=submit]")
await this.nextBeat

const enctype = (await this.searchParams).get("enctype")
const enctype = await this.getSearchParam("enctype")
this.assert.ok(enctype?.startsWith("multipart/form-data"), "submits a multipart/form-data request")
}

async "test standard GET form submission ignores enctype"() {
await this.clickSelector("#standard form[method=get][enctype] input[type=submit]")
await this.nextBeat

const enctype = (await this.searchParams).get("enctype")
const enctype = await this.getSearchParam("enctype")
this.assert.notOk(enctype, "GET form submissions ignore enctype")
}

async "test standard POST form submission without an enctype"() {
await this.clickSelector("#standard form[method=post].no-enctype input[type=submit]")
await this.nextBeat

const enctype = (await this.searchParams).get("enctype")
const enctype = await this.getSearchParam("enctype")
this.assert.ok(enctype?.startsWith("application/x-www-form-urlencoded"), "submits a application/x-www-form-urlencoded request")
}

async "test no-action form submission with single parameter"() {
await this.clickSelector("#no-action form.single input[type=submit]")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.equal(await await this.getSearchParam("query"), "1")

await this.clickSelector("#no-action form.single input[type=submit]")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.equal(await await this.getSearchParam("query"), "1")

await this.goToLocation("/src/tests/fixtures/form.html?query=2")
await this.clickSelector("#no-action form.single input[type=submit]")
await this.nextBody

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

async "test no-action form submission with multiple parameters"() {
await this.goToLocation("/src/tests/fixtures/form.html?query=2")
await this.clickSelector("#no-action form.multiple input[type=submit]")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.deepEqual(await this.getAllSearchParams("query"), [ "1", "2" ])

await this.clickSelector("#no-action form.multiple input[type=submit]")
await this.nextBody

this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
this.assert.deepEqual(await this.getAllSearchParams("query"), [ "1", "2" ])
}

async "test invalid form submission with unprocessable entity status"() {
await this.clickSelector("#reject form.unprocessable_entity input[type=submit]")
await this.nextBody
Expand Down Expand Up @@ -103,7 +148,7 @@ export class FormSubmissionTests extends TurboDriveTestCase {
await this.clickSelector("#submitter form[method=post]:not([enctype]) input[formenctype]")
await this.nextBeat

const enctype = (await this.searchParams).get("enctype")
const enctype = await this.getSearchParam("enctype")
this.assert.ok(enctype?.startsWith("multipart/form-data"), "submits a multipart/form-data request")
}

Expand Down
8 changes: 8 additions & 0 deletions src/tests/helpers/functional_test_case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ export class FunctionalTestCase extends InternTestCase {
return this.evaluate(() => location.search).then(search => new URLSearchParams(search))
}

async getSearchParam(key: string): Promise<string> {
return (await this.searchParams).get(key) || ""
}

async getAllSearchParams(key: string): Promise<string[]> {
return (await this.searchParams).getAll(key) || []
}

get hash(): Promise<string> {
return this.evaluate(() => location.hash)
}
Expand Down

0 comments on commit 9efa22d

Please sign in to comment.