diff --git a/.changeset/bright-rivers-sparkle.md b/.changeset/bright-rivers-sparkle.md new file mode 100644 index 000000000000..3076d719210e --- /dev/null +++ b/.changeset/bright-rivers-sparkle.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": patch +--- + +fix: respect HTML constraints for remote forms diff --git a/packages/kit/src/runtime/client/remote-functions/form.svelte.js b/packages/kit/src/runtime/client/remote-functions/form.svelte.js index 090fd273f971..4681dab69084 100644 --- a/packages/kit/src/runtime/client/remote-functions/form.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/form.svelte.js @@ -20,7 +20,8 @@ import { build_path_string, normalize_issue, serialize_binary_form, - BINARY_FORM_CONTENT_TYPE + BINARY_FORM_CONTENT_TYPE, + split_path } from '../../form-utils.js'; /** @@ -87,6 +88,9 @@ export function form(id) { /** @type {Record} */ let touched = {}; + /** @type {WeakSet} */ + let user_edited_text_controls = new WeakSet(); + let submitted = false; /** @@ -306,6 +310,34 @@ export function form(id) { return; } + const no_validate = clone(form).noValidate; // respects
+ const submitter_no_validate = + event.submitter && + /** @type {HTMLButtonElement | HTMLInputElement} */ (event.submitter).hasAttribute( + 'formnovalidate' + ); + + if (!no_validate && !submitter_no_validate) { + // reportValidity() triggers browser UI; returns false if invalid (minlength/maxlength/pattern/etc.) + if (!form.reportValidity()) { + event.preventDefault(); + return; + } + + const invalid_length_control = get_invalid_length_control( + form, + user_edited_text_controls + ); + if (invalid_length_control) { + event.preventDefault(); + // Browser validity can miss minlength/maxlength after a user edit if the + // value is later reapplied programmatically. Focus the control so the submit + // does not silently disappear when no native message is available. + invalid_length_control.focus(); + return; + } + } + event.preventDefault(); const form_data = new FormData(form, event.submitter); @@ -334,14 +366,26 @@ export function form(id) { element = form; touched = {}; + user_edited_text_controls = new WeakSet(); form.addEventListener('submit', onsubmit); /** @param {Event} e */ const handle_input = (e) => { - // strictly speaking it can be an HTMLTextAreaElement or HTMLSelectElement - // but that makes the types unnecessarily awkward - const element = /** @type {HTMLInputElement} */ (e.target); + const element = e.target; + if ( + !( + element instanceof HTMLInputElement || + element instanceof HTMLTextAreaElement || + element instanceof HTMLSelectElement + ) + ) { + return; + } + + if (element instanceof HTMLInputElement || element instanceof HTMLTextAreaElement) { + user_edited_text_controls.add(element); + } let name = element.name; if (!name) return; @@ -356,7 +400,7 @@ export function form(id) { if (is_array) { let value; - if (element.tagName === 'SELECT') { + if (element instanceof HTMLSelectElement) { value = Array.from( element.querySelectorAll('option:checked'), (e) => /** @type {HTMLOptionElement} */ (e).value @@ -386,13 +430,16 @@ export function form(id) { set_nested_value(input, name, value); } else if (is_file) { - if (DEV && element.multiple) { + const input_element = /** @type {HTMLInputElement} */ (element); + + if (DEV && input_element.multiple) { throw new Error( `Can only use the \`multiple\` attribute when \`name\` includes a \`[]\` suffix — consider changing "${name}" to "${name}[]"` ); } - const file = /** @type {HTMLInputElement & { files: FileList }} */ (element).files[0]; + const file = /** @type {HTMLInputElement & { files: FileList }} */ (input_element) + .files[0]; if (file) { set_nested_value(input, name, file); @@ -410,7 +457,9 @@ export function form(id) { set_nested_value( input, name, - element.type === 'checkbox' && !element.checked ? null : element.value + element instanceof HTMLInputElement && element.type === 'checkbox' && !element.checked + ? null + : element.value ); } @@ -427,6 +476,7 @@ export function form(id) { await tick(); input = convert_formdata(new FormData(form)); + user_edited_text_controls = new WeakSet(); }; form.addEventListener('reset', handle_reset); @@ -476,11 +526,13 @@ export function form(id) { (path, value) => { if (path.length === 0) { input = value; + clear_user_edited_text_controls(element, user_edited_text_controls); } else { deep_set(input, path.map(String), value); const key = build_path_string(path); touched[key] = true; + clear_user_edited_text_controls(element, user_edited_text_controls, key); } }, () => issues @@ -517,8 +569,15 @@ export function form(id) { /** @type {InternalRemoteFormIssue[]} */ let array = []; + let is_server_validation = false; const data = convert(form_data); + const html_constraint_issues = get_html_constraint_issues(element, { + include_untouched: includeUntouched, + submitted, + touched, + user_edited_text_controls + }); const validated = await preflight_schema?.['~standard'].validate(data); @@ -528,7 +587,7 @@ export function form(id) { if (validated?.issues) { array = validated.issues.map((issue) => normalize_issue(issue, false)); - } else if (!preflightOnly) { + } else if (!preflightOnly && html_constraint_issues.length === 0) { const response = await fetch(`${base}/${app_dir}/remote/${action_id_without_key}`, { method: 'POST', headers: { @@ -553,14 +612,25 @@ export function form(id) { devalue.parse(result.result, app.decoders) ); } + + is_server_validation = true; + } + + if (html_constraint_issues.length > 0) { + // eslint-disable-next-line svelte/prefer-svelte-reactivity + const html_constraint_names = new Set( + html_constraint_issues.map((issue) => issue.name) + ); + array = [ + ...array.filter((issue) => !html_constraint_names.has(issue.name)), + ...html_constraint_issues + ]; } if (!includeUntouched && !submitted) { array = array.filter((issue) => touched[issue.name]); } - const is_server_validation = !validated?.issues && !preflightOnly; - raw_issues = is_server_validation ? array : merge_with_server_issues(form_data, raw_issues, array); @@ -625,6 +695,148 @@ function clone(element) { return /** @type {T} */ (HTMLElement.prototype.cloneNode.call(element)); } +/** + * In some cases programmatic value updates can bypass minlength/maxlength checks during submit. + * Re-check text controls whose current value still came from direct user input. + * @param {HTMLFormElement} form + * @param {WeakSet} user_edited_text_controls + */ +function get_invalid_length_control(form, user_edited_text_controls) { + for (const element of form.elements) { + if (!(element instanceof HTMLInputElement || element instanceof HTMLTextAreaElement)) continue; + if (!element.willValidate || element.disabled || !element.name) continue; + if (!user_edited_text_controls.has(element)) continue; + + if (has_invalid_length(element)) return element; + } + + return null; +} + +/** + * @param {HTMLInputElement | HTMLTextAreaElement} element + */ +function has_invalid_length(element) { + const value = element.value; + const min_length = element.minLength; + const max_length = element.maxLength; + + if (value.length > 0 && min_length > -1 && value.length < min_length) return true; + if (max_length > -1 && value.length > max_length) return true; + return false; +} + +/** + * @param {HTMLFormElement} form + * @param {{ + * include_untouched: boolean, + * submitted: boolean, + * touched: Record, + * user_edited_text_controls: WeakSet + * }} options + * @returns {InternalRemoteFormIssue[]} + */ +function get_html_constraint_issues(form, options) { + /** @type {InternalRemoteFormIssue[]} */ + const issues = []; + // eslint-disable-next-line svelte/prefer-svelte-reactivity + const seen = new Set(); + + for (const element of form.elements) { + if ( + !( + element instanceof HTMLInputElement || + element instanceof HTMLTextAreaElement || + element instanceof HTMLSelectElement + ) + ) { + continue; + } + + if (!element.willValidate || element.disabled || !element.name) continue; + + const name = normalize_control_name(element.name); + const dedupe_key = + element instanceof HTMLInputElement && element.type === 'radio' ? `radio:${name}` : null; + if (dedupe_key && seen.has(dedupe_key)) continue; + if (!options.include_untouched && !options.submitted && !options.touched[name]) continue; + const invalid_length = + (element instanceof HTMLInputElement || element instanceof HTMLTextAreaElement) && + options.user_edited_text_controls.has(element) && + has_invalid_length(element); + if (!invalid_length && element.checkValidity()) continue; + + const parsed = parse_issue_path(name); + if (!parsed) continue; + + issues.push({ + name: parsed.name, + path: parsed.path, + message: element.validationMessage || 'Invalid value', + server: false + }); + + if (dedupe_key) { + seen.add(dedupe_key); + } + } + + return issues; +} + +/** + * @param {HTMLFormElement | null} form + * @param {WeakSet} user_edited_text_controls + * @param {string | null} [path] + */ +function clear_user_edited_text_controls(form, user_edited_text_controls, path = null) { + if (!form) return; + + for (const element of form.elements) { + if (!(element instanceof HTMLInputElement || element instanceof HTMLTextAreaElement)) { + continue; + } + + if (path !== null) { + const name = normalize_control_name(element.name); + if (!matches_path(name, path)) continue; + } + + user_edited_text_controls.delete(element); + } +} + +/** + * @param {string} name + */ +function normalize_control_name(name) { + if (name.endsWith('[]')) name = name.slice(0, -2); + return name.replace(/^[nb]:/, ''); +} + +/** + * @param {string} name + * @returns {{ name: string, path: Array } | null} + */ +function parse_issue_path(name) { + try { + return { + name, + path: split_path(name).map((segment) => (/^\d+$/.test(segment) ? Number(segment) : segment)) + }; + } catch { + return null; + } +} + +/** + * @param {string} name + * @param {string} path + */ +function matches_path(name, path) { + return name === path || name.startsWith(path + '.') || name.startsWith(path + '['); +} + /** * @param {FormData} form_data * @param {string} enctype diff --git a/packages/kit/test/apps/async/src/routes/remote/form/html-constraints/+page.svelte b/packages/kit/test/apps/async/src/routes/remote/form/html-constraints/+page.svelte new file mode 100644 index 000000000000..0570f40196c9 --- /dev/null +++ b/packages/kit/test/apps/async/src/routes/remote/form/html-constraints/+page.svelte @@ -0,0 +1,38 @@ + + + + + + + + +

{validate_code.result || ''}

+ +
validate_code_preflight.validate({ preflightOnly: true })} +> + + +
+ +

{validate_code_preflight.fields.code.issues()?.length ?? 0}

diff --git a/packages/kit/test/apps/async/src/routes/remote/form/html-constraints/form.remote.ts b/packages/kit/test/apps/async/src/routes/remote/form/html-constraints/form.remote.ts new file mode 100644 index 000000000000..66a4fa1ab2e9 --- /dev/null +++ b/packages/kit/test/apps/async/src/routes/remote/form/html-constraints/form.remote.ts @@ -0,0 +1,20 @@ +import { form } from '$app/server'; +import * as v from 'valibot'; + +export const validate_code = form( + v.object({ + code: v.string() + }), + async (data) => { + return data.code; + } +); + +export const validate_code_preflight = form( + v.object({ + code: v.string() + }), + async (data) => { + return data.code; + } +); diff --git a/packages/kit/test/apps/async/test/test.js b/packages/kit/test/apps/async/test/test.js index c5af41c86d31..cee2824e6554 100644 --- a/packages/kit/test/apps/async/test/test.js +++ b/packages/kit/test/apps/async/test/test.js @@ -361,6 +361,124 @@ test.describe('remote functions', () => { await expect(issues).not.toContainText('a is too short'); }); + test('form respects HTML constraints before submitting remote requests', async ({ + page, + javaScriptEnabled + }) => { + if (!javaScriptEnabled) return; + + await page.goto('/remote/form/html-constraints'); + + let request_count = 0; + + /** @param {import('@playwright/test').Request} request */ + const handler = (request) => { + if (request.url().includes('/_app/remote') && request.method() === 'POST') { + request_count += 1; + } + }; + + page.on('request', handler); + + const input = page.locator('[data-submit] input[name="code"]'); + const submit = page.locator('[data-submit] button[type="submit"]'); + const result = page.locator('#result'); + + await input.focus(); + await input.pressSequentially('1'); + await submit.click(); + await expect(result).toHaveText(''); + await expect( + page.waitForRequest( + (request) => request.url().includes('/_app/remote') && request.method() === 'POST', + { timeout: 200 } + ) + ).rejects.toThrow(); + + await input.fill(''); + await input.pressSequentially('123456'); + await submit.click(); + await expect(result).toHaveText('123456'); + expect(request_count).toBe(1); + + page.off('request', handler); + }); + + test('form validate({ preflightOnly: true }) respects HTML constraints on dirty fields', async ({ + page, + javaScriptEnabled + }) => { + if (!javaScriptEnabled) return; + + await page.goto('/remote/form/html-constraints'); + + let request_count = 0; + + /** @param {import('@playwright/test').Request} request */ + const handler = (request) => { + if (request.url().includes('/_app/remote') && request.method() === 'POST') { + request_count += 1; + } + }; + + page.on('request', handler); + + const input = page.locator('[data-preflight] input[name="code"]'); + const issue_count = page.locator('#preflight-issue-count'); + + await input.focus(); + await input.pressSequentially('1'); + await input.blur(); + await expect(issue_count).toHaveText('1'); + expect(request_count).toBe(0); + + await input.fill('123456'); + await input.blur(); + await expect(issue_count).toHaveText('0'); + expect(request_count).toBe(0); + + page.off('request', handler); + }); + + test('form allows programmatic values that only fail length constraints', async ({ + page, + javaScriptEnabled + }) => { + if (!javaScriptEnabled) return; + + await page.goto('/remote/form/html-constraints'); + + const input = page.locator('[data-submit] input[name="code"]'); + const set = page.locator('[data-programmatic-submit]'); + const submit = page.locator('[data-submit] button[type="submit"]'); + const result = page.locator('#result'); + const request = page.waitForRequest( + (request) => request.url().includes('/_app/remote') && request.method() === 'POST' + ); + + await set.click(); + await expect(input).toHaveValue('1'); + await submit.click(); + await request; + await expect(result).toHaveText('1'); + }); + + test('form validate({ preflightOnly: true }) ignores programmatic length values', async ({ + page, + javaScriptEnabled + }) => { + if (!javaScriptEnabled) return; + + await page.goto('/remote/form/html-constraints'); + + const input = page.locator('[data-preflight] input[name="code"]'); + const issue_count = page.locator('#preflight-issue-count'); + + await page.locator('[data-programmatic-preflight]').click(); + await expect(input).toHaveValue('1'); + await expect(issue_count).toHaveText('0'); + }); + test('form validate works', async ({ page, javaScriptEnabled }) => { if (!javaScriptEnabled) return;