Skip to content

Commit 0fd2d55

Browse files
elliott-with-the-longest-name-on-githubgtm-nayanbenmccannRich HarrisRich-Harris
authored
feat: Warn users when submitting forms with files but no enctype="multipart/form-data" (#9888)
* fix: Package name keeps me from filtering with pnpm * feat: Warn users when submitting a form containing a file without the correct enctype * changeset * Update packages/kit/src/runtime/app/forms.js Co-authored-by: gtmnayan <[email protected]> * style * moar style tweaks * better test skip * Update .changeset/tasty-llamas-relate.md Co-authored-by: Ben McCann <[email protected]> * DRY out * only warn once per submit * Update .changeset/tasty-llamas-relate.md Co-authored-by: Rich Harris <[email protected]> --------- Co-authored-by: gtmnayan <[email protected]> Co-authored-by: Ben McCann <[email protected]> Co-authored-by: Rich Harris <[email protected]> Co-authored-by: Rich Harris <[email protected]>
1 parent a2f23ca commit 0fd2d55

File tree

5 files changed

+59
-8
lines changed

5 files changed

+59
-8
lines changed

.changeset/tasty-llamas-relate.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
feat: warn users when enhancing forms with files but no `enctype="multipart/form-data"`

packages/kit/src/runtime/app/forms.js

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,20 @@ function warn_on_access(old_name, new_name, call_location) {
2828
);
2929
}
3030

31+
/**
32+
* Shallow clone an element, so that we can access e.g. `form.action` without worrying
33+
* that someone has added an `<input name="action">` (https://github.com/sveltejs/kit/issues/7593)
34+
* @template {HTMLElement} T
35+
* @param {T} element
36+
* @returns {T}
37+
*/
38+
function clone(element) {
39+
return /** @type {T} */ (HTMLElement.prototype.cloneNode.call(element));
40+
}
41+
3142
/** @type {import('$app/forms').enhance} */
3243
export function enhance(form_element, submit = () => {}) {
33-
if (
34-
DEV &&
35-
/** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form_element))
36-
.method !== 'post'
37-
) {
44+
if (DEV && clone(form_element).method !== 'post') {
3845
throw new Error('use:enhance can only be used on <form> fields with method="POST"');
3946
}
4047

@@ -71,15 +78,25 @@ export function enhance(form_element, submit = () => {}) {
7178

7279
const action = new URL(
7380
// We can't do submitter.formAction directly because that property is always set
74-
// We do cloneNode for avoid DOM clobbering - https://github.com/sveltejs/kit/issues/7593
7581
event.submitter?.hasAttribute('formaction')
7682
? /** @type {HTMLButtonElement | HTMLInputElement} */ (event.submitter).formAction
77-
: /** @type {HTMLFormElement} */ (HTMLFormElement.prototype.cloneNode.call(form_element))
78-
.action
83+
: clone(form_element).action
7984
);
8085

8186
const form_data = new FormData(form_element);
8287

88+
if (DEV && clone(form_element).enctype !== 'multipart/form-data') {
89+
for (const value of form_data.values()) {
90+
if (value instanceof File) {
91+
// TODO 2.0: Upgrade to `throw Error`
92+
console.warn(
93+
'Your form contains <input type="file"> fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.'
94+
);
95+
break;
96+
}
97+
}
98+
}
99+
83100
const submitter_name = event.submitter?.getAttribute('name');
84101
if (submitter_name) {
85102
form_data.append(submitter_name, event.submitter?.getAttribute('value') ?? '');
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export const actions = {
2+
default: async ({ request }) => {
3+
const data = await request.formData();
4+
console.log(data.get('file'));
5+
}
6+
};
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
import { enhance } from '$app/forms';
3+
</script>
4+
5+
<form method="POST" use:enhance>
6+
<input name="file" type="file" />
7+
<button type="submit">upload</button>
8+
</form>

packages/kit/test/apps/basics/test/test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,21 @@ test.describe('Matchers', () => {
828828
});
829829

830830
test.describe('Actions', () => {
831+
test('Submitting a form with a file input but no enctype="multipart/form-data" logs a warning', async ({
832+
page,
833+
javaScriptEnabled
834+
}) => {
835+
test.skip(!javaScriptEnabled, 'Skip when JavaScript is disabled');
836+
test.skip(!process.env.DEV, 'Skip when not in dev mode');
837+
await page.goto('/actions/file-without-enctype');
838+
const log_promise = page.waitForEvent('console');
839+
await page.click('button');
840+
const log = await log_promise;
841+
expect(log.text()).toBe(
842+
'Your form contains <input type="file"> fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.'
843+
);
844+
});
845+
831846
test(`Accessing v2 deprecated properties results in a warning log`, async ({
832847
page,
833848
javaScriptEnabled

0 commit comments

Comments
 (0)