Compare commits

...

2 Commits

Author SHA1 Message Date
jaeone94
ac64dc0469 fix: un-escape special characters in error tab aria-labels
The initial fix un-escaped the visible error-tab text, but three
:aria-label bindings re-interpolated the now-raw node/model name
through the global t(), re-introducing HTML entities into the
accessibility tree. Pass escapeParameter: false at those sites
(ErrorGroupList info/locate, MissingMediaCard locate) so the
accessible name matches the visible label.

Add aria-label regression tests for ErrorGroupList and MissingMediaCard,
a catalog-key existence guard on the model-name test, and align the
shared test i18n fixture with production so the guards actually exercise
escaping.
2026-07-16 09:00:46 +09:00
jaeone94
905bc50129 fix: stop double-escaping special characters in error tab copy
Error catalog resolvers pass node and model names to vue-i18n's
t(key, params). Because escapeParameter is enabled globally, those
params are HTML-escaped, and the resolved strings are then rendered
through Vue text interpolation, which escapes again at the render
boundary. Names containing &, <, or > therefore surfaced as literal
&amp;, &lt;, &gt; in the error tab.

Pass escapeParameter: false at the two catalog translation call sites
so escaping happens exactly once, at the Vue render boundary. XSS
protection is unchanged since the render boundary still escapes.
2026-07-15 12:10:49 +09:00
8 changed files with 199 additions and 12 deletions

View File

@@ -236,4 +236,27 @@ describe('ErrorGroupList selection emphasis', () => {
expect(strip).toHaveTextContent('2 nodes — 2 errors')
})
})
it('preserves special characters in execution item accessible names', () => {
const nodeDisplayName = 'A & B <C>'
vi.mocked(getNodeByExecutionId).mockImplementation((_, nodeId) =>
fromAny<LGraphNode, unknown>(
String(nodeId) === '1'
? { ...SAMPLER_NODE, title: nodeDisplayName }
: LOADER_NODE
)
)
const pinia = createPinia()
seedTwoErrorGroups(pinia)
renderList(pinia)
expect(
screen.getByRole('button', { name: /Info for A & B <C>/ })
).toBeInTheDocument()
expect(
screen.getByRole('button', { name: /Locate A & B <C>/ })
).toBeInTheDocument()
expect(screen.queryAllByLabelText(/&(?:amp|lt|gt);/)).toHaveLength(0)
})
})

View File

@@ -239,7 +239,11 @@
)
"
:aria-label="
t('rightSidePanel.infoFor', { item: item.label })
t(
'rightSidePanel.infoFor',
{ item: item.label },
{ escapeParameter: false }
)
"
:aria-controls="getExecutionItemDetailId(item.key)"
:aria-expanded="isExecutionItemDetailExpanded(item.key)"
@@ -253,9 +257,13 @@
size="icon-sm"
class="size-8 shrink-0 text-muted-foreground hover:text-base-foreground focus-visible:ring-inset"
:aria-label="
t('rightSidePanel.locateNodeFor', {
item: item.label
})
t(
'rightSidePanel.locateNodeFor',
{
item: item.label
},
{ escapeParameter: false }
)
"
@click.stop="handleLocateNode(item.nodeId)"
>

View File

@@ -6,8 +6,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import { createI18n } from 'vue-i18n'
import ErrorNodeCard from './ErrorNodeCard.vue'
import type { ErrorCardData } from './types'
import { resolveRunErrorMessage } from '@/platform/errorCatalog/errorMessageResolver'
import { createNodeExecutionId } from '@/types/nodeIdentification'
import { toNodeId } from '@/types/nodeId'
import { validationError } from '@/utils/__tests__/nodeErrorHelpers'
const mockGetLogs = vi.fn(() => Promise.resolve('mock server logs'))
const mockSerialize = vi.fn(() => ({ nodes: [] }))
@@ -100,7 +102,7 @@ describe('ErrorNodeCard.vue', () => {
const user = userEvent.setup()
const onCopyToClipboard = vi.fn()
const onLocateNode = vi.fn()
render(ErrorNodeCard, {
const { container } = render(ErrorNodeCard, {
props: { card, onCopyToClipboard, onLocateNode },
global: {
plugins: [
@@ -142,7 +144,7 @@ describe('ErrorNodeCard.vue', () => {
}
}
})
return { user, onCopyToClipboard, onLocateNode }
return { container, user, onCopyToClipboard, onLocateNode }
}
async function toggleRuntimeDetails(
@@ -185,6 +187,32 @@ describe('ErrorNodeCard.vue', () => {
}
}
function makeValidationErrorCard(nodeDisplayName: string): ErrorCardData {
const error = validationError(
'required_input_missing',
'model',
{},
'Required input is missing',
'model'
)
return {
id: `validation-${++cardIdCounter}`,
title: 'Validation error',
errors: [
{
message: error.message,
details: error.details,
...resolveRunErrorMessage({
kind: 'node_validation',
error,
nodeDisplayName
})
}
]
}
}
it('shows runtime details by default and can collapse them', async () => {
const reportText =
'# ComfyUI Error Report\n## System Information\n- OS: Linux'
@@ -280,6 +308,21 @@ describe('ErrorNodeCard.vue', () => {
).not.toBeInTheDocument()
})
it('renders catalog copy with literal special characters', () => {
const { container } = renderCard(makeValidationErrorCard('Foo & Bar <C>'))
expect(container.textContent).toContain('Foo & Bar <C>')
expect(container.textContent).not.toMatch(/&(?:amp|lt|gt);/)
})
it('renders script-like node names as literal text', () => {
const { container } = renderCard(
makeValidationErrorCard('<script>x</script>')
)
expect(container.textContent).toContain('<script>x</script>')
})
it('copies enriched report when copy button is clicked for runtime error', async () => {
const reportText = '# Full Report Content'
mockGenerateErrorReport.mockReturnValue(reportText)

View File

@@ -34,6 +34,7 @@ export function setupTestPinia() {
export const testI18n = createI18n({
legacy: false,
locale: 'en',
escapeParameter: true,
messages: { en: enMessages }
})

View File

@@ -15,7 +15,8 @@ export function translateCatalogMessage(
fallback: string,
params?: CatalogParams
): string {
if (te(key)) return params ? t(key, params) : t(key)
if (te(key))
return params ? t(key, params, { escapeParameter: false }) : t(key)
if (!params) return fallback
return fallback.replace(/\{(\w+)\}/g, (match, paramName) =>
@@ -28,7 +29,8 @@ export function translateOptionalCatalogMessage(
fallback?: string,
params?: CatalogParams
): string | undefined {
if (te(key)) return params ? t(key, params) : t(key)
if (te(key))
return params ? t(key, params, { escapeParameter: false }) : t(key)
return fallback?.trim() ? fallback : undefined
}

View File

@@ -10,7 +10,7 @@ import type { ExecutionErrorWsMessage } from '@/schemas/apiSchema'
import type { MissingMediaGroup } from '@/platform/missingMedia/types'
import type { MissingModelGroup } from '@/platform/missingModel/types'
import type { MissingNodeType } from '@/types/comfy'
import { i18n } from '@/i18n'
import { i18n, te } from '@/i18n'
function nodeValidationError(
type: string,
@@ -131,6 +131,35 @@ describe('errorMessageResolver', () => {
})
})
it('preserves special characters in catalog copy for node names', () => {
const nodeDisplayName = 'A & B <C>'
const result = resolveRunErrorMessage({
kind: 'execution',
error: executionError('ImageDownloadError', 'Failed to validate images'),
nodeDisplayName
})
const interpolatedCopy = [
result.displayItemLabel,
result.toastMessage
].join(' ')
expect(interpolatedCopy).toContain(nodeDisplayName)
expect(interpolatedCopy).not.toMatch(/&(?:amp|lt|gt);/)
expect(interpolatedCopy).toContain("couldn't be loaded")
})
it('preserves special characters in catalog details copy for node names', () => {
const nodeDisplayName = 'A & B <C>'
const result = resolveRunErrorMessage({
kind: 'node_validation',
error: requiredInputMissing('model'),
nodeDisplayName
})
expect(result.displayDetails).toContain(nodeDisplayName)
expect(result.displayDetails).not.toMatch(/&(?:amp|lt|gt);/)
})
it('uses catalog fallbacks when required_input_missing lacks node or input labels', () => {
expect(
resolveRunErrorMessage({
@@ -1542,6 +1571,22 @@ describe('errorMessageResolver', () => {
})
})
it('preserves special characters in catalog copy for model names', () => {
const modelName = 'sd&xl<v2>.safetensors'
expect(
te('errorCatalog.missingErrors.missing_model.toastTitleOneOss')
).toBe(true)
const result = resolveMissingErrorMessage({
kind: 'missing_model',
groups: missingModelGroups(modelName),
count: 1,
isCloud: false
})
expect(result.toastTitle).toContain(modelName)
expect(result.toastTitle).not.toMatch(/&(?:amp|lt|gt);/)
})
it('resolves missing media group display and toast copy', () => {
const groups: MissingMediaGroup[] = [
{

View File

@@ -0,0 +1,61 @@
import { render, screen } from '@testing-library/vue'
import { describe, expect, it, vi } from 'vitest'
import { testI18n } from '@/components/searchbox/v2/__test__/testUtils'
import type { MissingMediaGroup } from '@/platform/missingMedia/types'
import { getNodeByExecutionId } from '@/utils/graphTraversalUtil'
import { createMockLGraphNode } from '@/utils/__tests__/litegraphTestUtils'
import MissingMediaCard from './MissingMediaCard.vue'
vi.mock('@/scripts/app', () => ({
app: { rootGraph: {} }
}))
vi.mock('@/utils/graphTraversalUtil', () => ({
getNodeByExecutionId: vi.fn()
}))
describe('MissingMediaCard', () => {
it('preserves special characters in the locate button accessible name', () => {
vi.mocked(getNodeByExecutionId).mockReturnValue(
createMockLGraphNode({ title: 'A & B <C>' })
)
const missingMediaGroups: MissingMediaGroup[] = [
{
mediaType: 'image',
items: [
{
name: 'image.png',
mediaType: 'image',
representative: {
nodeId: '1',
nodeType: 'LoadImage',
widgetName: 'image',
mediaType: 'image',
name: 'image.png',
isMissing: true
},
referencingNodes: [
{
nodeId: '1',
nodeType: 'LoadImage',
widgetName: 'image'
}
]
}
]
}
]
render(MissingMediaCard, {
props: { missingMediaGroups },
global: { plugins: [testI18n] }
})
expect(
screen.getByRole('button', { name: 'Locate A & B <C> - image' })
).toBeInTheDocument()
expect(screen.queryAllByLabelText(/&(?:amp|lt|gt);/)).toHaveLength(0)
})
})

View File

@@ -40,9 +40,13 @@
size="icon-sm"
class="size-8 shrink-0 text-muted-foreground hover:text-base-foreground focus-visible:ring-inset"
:aria-label="
t('rightSidePanel.locateNodeFor', {
item: item.displayItemLabel
})
t(
'rightSidePanel.locateNodeFor',
{
item: item.displayItemLabel
},
{ escapeParameter: false }
)
"
@click.stop="emit('locateNode', item.nodeId)"
>