mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
perf: decode the cors path only when the fallback needs it
This commit is contained in:
@@ -192,7 +192,10 @@ async fn conditional_cors_middleware(
|
||||
let origin = req.headers().get(http::header::ORIGIN).cloned();
|
||||
// Owned before `next.run` consumes the request. `&Request` is not `Send`
|
||||
// (`Body` is not `Sync`), so nothing borrowed from it can cross the await.
|
||||
let lookup = cors_lookup_method(&req).zip(cors_lookup_path(req.uri().path()));
|
||||
// The URI is carried rather than the decoded path: cloning it is a refcount
|
||||
// bump, while decoding allocates, and only the fallback below ever needs it.
|
||||
let lookup_method = cors_lookup_method(&req);
|
||||
let uri = req.uri().clone();
|
||||
|
||||
let resolved = ResolvedCorsPolicy::default();
|
||||
req.extensions_mut().insert(resolved.clone());
|
||||
@@ -208,7 +211,7 @@ async fn conditional_cors_middleware(
|
||||
// that failed before reaching the publish, authentication included. No
|
||||
// runnable produced this body, so reading the cache cannot contradict
|
||||
// anything.
|
||||
None => match lookup {
|
||||
None => match lookup_method.zip(cors_lookup_path(uri.path())) {
|
||||
Some((method, path)) => {
|
||||
resolve_cors_decision(&db, method, &path, origin.as_ref()).await
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { ButtonType } from './common/button/model'
|
||||
import { allowedOriginsError, parseAllowedOriginsSetting } from './triggers/http/utils'
|
||||
import { allowedOriginsSettingError } from './triggers/http/utils'
|
||||
import { z } from 'zod'
|
||||
import { writable } from 'svelte/store'
|
||||
|
||||
@@ -275,8 +275,7 @@ export const settings: Record<string, Setting[]> = {
|
||||
'Each origin must be visible ASCII with no comma, and there can be at most 100 of them. null is not allowed, since every sandboxed iframe sends it.',
|
||||
// The same check the API applies, so a value it would refuse cannot be
|
||||
// saved here and then silently drop to no restriction at the next boot.
|
||||
isValid: (value: string | undefined) =>
|
||||
allowedOriginsError(parseAllowedOriginsSetting(value)) === undefined,
|
||||
isValid: (value: unknown) => allowedOriginsSettingError(value) === undefined,
|
||||
ee_only: '',
|
||||
hideInQuickSetup: true
|
||||
},
|
||||
|
||||
@@ -27,13 +27,18 @@ export function parseAllowedOrigins(raw: string): string[] {
|
||||
* risk. `null` is the exception, since it is what every sandboxed iframe sends.
|
||||
*/
|
||||
export function allowedOriginRejection(origin: string): string | undefined {
|
||||
// Same order as `validate_allowed_origins`, so the same entry draws the same
|
||||
// message on both sides rather than only the same verdict.
|
||||
if (origin === '*') return undefined
|
||||
if (origin === '') return 'An origin must not be empty'
|
||||
if (origin.length > MAX_ALLOWED_ORIGIN_LEN)
|
||||
return `'${origin.slice(0, 40)}…' is longer than any origin a browser sends`
|
||||
if (origin.includes(','))
|
||||
return `'${origin}' must not contain a comma, which separates entries`
|
||||
if (origin.toLowerCase() === 'null')
|
||||
return `'null' is what a sandboxed iframe sends, so it would allow any page that can open one`
|
||||
if (!/^[\x21-\x7e]+$/.test(origin))
|
||||
return `'${origin}' must contain only visible ASCII, with no whitespace`
|
||||
if (origin.length > MAX_ALLOWED_ORIGIN_LEN)
|
||||
return `'${origin.slice(0, 40)}…' is longer than any origin a browser sends`
|
||||
return undefined
|
||||
}
|
||||
|
||||
@@ -76,15 +81,38 @@ export function allowedOriginWarning(origin: string): string | undefined {
|
||||
// purpose: browsers send origins this cannot anticipate, `chrome-extension`
|
||||
// and IPv6 literals among them, and a warning that cries wolf on a working
|
||||
// origin is worse than one that stays quiet.
|
||||
const port = rest.startsWith('[')
|
||||
? rest.slice(rest.indexOf(']') + 1).replace(/^:/, '')
|
||||
: rest.split(':')[1]
|
||||
if (rest.startsWith(':')) return `'${origin}' has no host`
|
||||
if (port !== undefined && port !== '' && !(/^[0-9]{1,5}$/.test(port) && Number(port) <= 65535))
|
||||
const portStart = rest.startsWith('[') ? rest.indexOf(']') + 1 : rest.indexOf(':')
|
||||
// A trailing colon is a port, an empty one — distinct from having none.
|
||||
const port = portStart > 0 && rest[portStart] === ':' ? rest.slice(portStart + 1) : undefined
|
||||
if (port !== undefined && !(/^[0-9]{1,5}$/.test(port) && Number(port) <= 65535))
|
||||
return `'${origin}' has a port no browser can send`
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* What the settings API would refuse, mirroring `parse_allowed_origins_setting`
|
||||
* in windmill-common.
|
||||
*
|
||||
* Distinct from reading the setting for display: that drops entries it cannot
|
||||
* use, while this has to report them, or a shape only the YAML editor can
|
||||
* produce would pass here and come back as a 400 on save.
|
||||
*/
|
||||
export function allowedOriginsSettingError(setting: unknown): string | undefined {
|
||||
let origins: string[]
|
||||
if (setting == null || typeof setting === 'string') {
|
||||
origins = parseAllowedOrigins(typeof setting === 'string' ? setting : '')
|
||||
} else if (Array.isArray(setting)) {
|
||||
if (setting.some((entry) => typeof entry !== 'string')) return 'Entries must be strings'
|
||||
// Not filtered for empties, unlike the comma-separated form, where a
|
||||
// trailing separator is a typing artifact rather than an entry.
|
||||
origins = setting.map((entry) => (entry as string).trim())
|
||||
} else {
|
||||
return 'Expected a comma-separated string or a list of strings'
|
||||
}
|
||||
return allowedOriginsError(origins)
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the instance-default setting, mirroring `parse_allowed_origins_setting`
|
||||
* in windmill-common: the settings UI writes a comma-separated string, but the
|
||||
|
||||
Reference in New Issue
Block a user