-
-
Notifications
You must be signed in to change notification settings - Fork 24.5k
fix(tools): validate Tool Icon Source URL and default icon fallbacks (#6369) #6376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { StatusCodes } from 'http-status-codes' | ||
| import { InternalFlowiseError } from '../errors/internalFlowiseError' | ||
|
|
||
| /** | ||
| * Normalize optional tool iconSrc from the API: empty clears (null); non-empty must be http(s). | ||
| * Returns undefined when the client omitted the field (partial updates). | ||
| */ | ||
| export function normalizeOptionalToolIconSrc(iconSrc: unknown): string | null | undefined { | ||
| if (iconSrc === undefined) return undefined | ||
| if (iconSrc === null) return null | ||
| if (typeof iconSrc !== 'string') { | ||
| throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, `Error: tool icon source must be a string, http(s) URL, or empty.`) | ||
| } | ||
| const trimmed = iconSrc.trim() | ||
| if (!trimmed) return null | ||
| try { | ||
| const u = new URL(trimmed) | ||
| if (u.protocol !== 'http:' && u.protocol !== 'https:') { | ||
| throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, `Error: tool icon source must be an http or https URL.`) | ||
| } | ||
| return u.href | ||
| } catch (e) { | ||
| if (e instanceof InternalFlowiseError) throw e | ||
| throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, `Error: tool icon source must be a valid http or https URL.`) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /** | ||
| * Custom Tool icon source: optional field; when set, must be an absolute http(s) URL. | ||
| * @param {string|undefined|null} value | ||
| * @returns {boolean} | ||
| */ | ||
| export function isOptionalHttpOrHttpsToolIconUrl(value) { | ||
| if (value == null) return true | ||
| const v = String(value).trim() | ||
| if (!v) return true | ||
| try { | ||
| const u = new URL(v) | ||
| return u.protocol === 'http:' || u.protocol === 'https:' | ||
| } catch { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param {string|undefined|null} value | ||
| * @returns {string|null} | ||
| */ | ||
| export function getValidHttpOrHttpsToolIconUrl(value) { | ||
| if (value == null) return null | ||
| const v = String(value).trim() | ||
| if (!v) return null | ||
| try { | ||
| const u = new URL(v) | ||
| if (u.protocol !== 'http:' && u.protocol !== 'https:') return null | ||
| return u.href | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Resolves a value suitable for CSS `background-image: url(...)`, or null to use color/default fallback. | ||
| * Accepts http(s) URLs, root-relative paths, and data URLs; rejects plain invalid strings like "abc". | ||
| * @param {string|undefined|null} iconSrc | ||
| * @returns {string|null} | ||
| */ | ||
| export function getItemCardIconBackgroundUrl(iconSrc) { | ||
| if (iconSrc == null) return null | ||
| const t = String(iconSrc).trim() | ||
| if (!t) return null | ||
| const http = getValidHttpOrHttpsToolIconUrl(t) | ||
| if (http) return http | ||
| if (t.startsWith('/') || t.startsWith('./')) return t | ||
| if (t.toLowerCase().startsWith('data:')) return t | ||
| return null | ||
| } | ||
|
Comment on lines
+41
to
+50
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a slight discrepancy between the UI rendering logic and the validation logic. References
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Node.js,
new URL(trimmed)will throw aTypeErrorif the input is not an absolute URL (e.g., just a domain likeexample.com). While thecatchblock correctly handles this by throwing aBAD_REQUESTerror, it's worth noting that this implementation strictly enforces absolute URLs. This is consistent with the goal of validating the icon source and promotes fail-fast behavior for invalid external data types.References