From 863473783dadc637a8418d4ffb52ee9d406f8b8d Mon Sep 17 00:00:00 2001 From: PathGao <42336971+PathGao@users.noreply.github.com> Date: Thu, 30 Jul 2026 02:51:40 +0800 Subject: [PATCH] fix(frontend): preserve cancellation and reject invalid query data (#6143) * fix(frontend): preserve request cancellation and schema failures * fix(frontend): limit schema failures to query boundaries * fix(frontend): keep invalid settings recoverable Keep settings payload validation tolerant so values accepted by the backend remain editable, while paged clients still fail closed. Add an AbortSignal.any fallback and make timeout tests event-driven. --------- Co-authored-by: PathGao --- frontend/src/api/http-init.ts | 26 ++++++- frontend/src/hooks/useClients.ts | 2 +- frontend/src/test/http-init.test.tsx | 83 +++++++++++++++++++++ frontend/src/test/use-all-settings.test.tsx | 28 +++++++ frontend/src/test/zodValidate.test.ts | 41 ++++++++++ frontend/src/utils/zodValidate.ts | 6 ++ 6 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 frontend/src/test/use-all-settings.test.tsx create mode 100644 frontend/src/test/zodValidate.test.ts diff --git a/frontend/src/api/http-init.ts b/frontend/src/api/http-init.ts index 2261d0d90..5a638523c 100644 --- a/frontend/src/api/http-init.ts +++ b/frontend/src/api/http-init.ts @@ -88,6 +88,28 @@ function encodeForm(data: unknown): string { return parts.join('&'); } +function appendQuery(url: string, query: string): string { + if (query === '') return url; + const hashIndex = url.indexOf('#'); + const path = hashIndex === -1 ? url : url.slice(0, hashIndex); + const hash = hashIndex === -1 ? '' : url.slice(hashIndex); + const hasQuery = path.includes('?'); + const separator = !hasQuery ? '?' : path.endsWith('?') || path.endsWith('&') ? '' : '&'; + return `${path}${separator}${query}${hash}`; +} + +function requestSignal(options: HttpRequestOptions): AbortSignal | undefined { + if (!options.timeout) return options.signal; + const timeout = AbortSignal.timeout(options.timeout); + if (!options.signal) return timeout; + if (typeof AbortSignal.any === 'function') return AbortSignal.any([options.signal, timeout]); + const controller = new AbortController(); + const abort = () => controller.abort(); + options.signal.addEventListener('abort', abort, { once: true }); + timeout.addEventListener('abort', abort, { once: true }); + return controller.signal; +} + async function performFetch( method: string, url: string, @@ -121,8 +143,8 @@ async function performFetch( } const query = encodeForm(options.params); - const fullUrl = basePathPrefix + url + (query ? `?${query}` : ''); - const signal = options.timeout ? AbortSignal.timeout(options.timeout) : options.signal; + const fullUrl = basePathPrefix + appendQuery(url, query); + const signal = requestSignal(options); return fetch(fullUrl, { method: upper, headers, body, credentials: 'same-origin', signal }); } diff --git a/frontend/src/hooks/useClients.ts b/frontend/src/hooks/useClients.ts index 9a094b2da..a23ccca96 100644 --- a/frontend/src/hooks/useClients.ts +++ b/frontend/src/hooks/useClients.ts @@ -155,7 +155,7 @@ async function fetchClientPage(params: ClientQueryParams): Promise { expect(initOf().signal).toBeInstanceOf(AbortSignal); }); + + it('preserves a caller cancellation signal when a timeout is set', async () => { + http.setupHttp(); + fetchMock.mockResolvedValue(okEnvelope()); + const controller = new AbortController(); + + await http.httpRequest('GET', '/x', undefined, { timeout: 1_000, signal: controller.signal }); + controller.abort(); + + expect(initOf().signal?.aborted).toBe(true); + }); + + it('preserves both cancellation paths when AbortSignal.any is unavailable', async () => { + const timeout = AbortSignal.timeout.bind(AbortSignal); + vi.resetModules(); + vi.stubGlobal('AbortSignal', { timeout }); + http = await import('@/api/http-init'); + http.setupHttp(); + fetchMock.mockResolvedValue(okEnvelope()); + const controller = new AbortController(); + + await http.httpRequest('GET', '/x', undefined, { timeout: 1_000, signal: controller.signal }); + controller.abort(); + + expect(initOf().signal?.aborted).toBe(true); + }); + + it('times out when AbortSignal.any is unavailable', async () => { + const timeout = AbortSignal.timeout.bind(AbortSignal); + vi.resetModules(); + vi.stubGlobal('AbortSignal', { timeout }); + http = await import('@/api/http-init'); + http.setupHttp(); + fetchMock.mockResolvedValue(okEnvelope()); + const controller = new AbortController(); + + await http.httpRequest('GET', '/x', undefined, { timeout: 20, signal: controller.signal }); + const signal = initOf().signal as AbortSignal; + await new Promise((resolve) => signal.addEventListener('abort', () => resolve(), { once: true })); + + expect(signal.aborted).toBe(true); + }); + + it('aborts on the timeout when a caller signal is present', async () => { + http.setupHttp(); + fetchMock.mockResolvedValue(okEnvelope()); + const controller = new AbortController(); + + await http.httpRequest('GET', '/x', undefined, { timeout: 20, signal: controller.signal }); + const signal = initOf().signal as AbortSignal; + await new Promise((resolve) => { + if (signal.aborted) { + resolve(); + return; + } + signal.addEventListener('abort', () => resolve(), { once: true }); + }); + + expect(signal.aborted).toBe(true); + }); + + it.each([ + ['/x?keep=1', '/x?keep=1&added=yes'], + ['/x?', '/x?added=yes'], + ['/x#frag', '/x?added=yes#frag'], + ['/x?keep=1#frag', '/x?keep=1&added=yes#frag'], + ])('appends encoded params to %s', async (url, expected) => { + http.setupHttp(); + fetchMock.mockResolvedValue(okEnvelope()); + + await http.httpRequest('GET', url, undefined, { params: { added: 'yes' } }); + + expect(urlOf()).toBe(expected); + }); + + it('preserves the URL when no params are supplied', async () => { + http.setupHttp(); + fetchMock.mockResolvedValue(okEnvelope()); + + await http.httpRequest('GET', '/x?keep=1#frag'); + + expect(urlOf()).toBe('/x?keep=1#frag'); + }); }); diff --git a/frontend/src/test/use-all-settings.test.tsx b/frontend/src/test/use-all-settings.test.tsx new file mode 100644 index 000000000..f4c50c040 --- /dev/null +++ b/frontend/src/test/use-all-settings.test.tsx @@ -0,0 +1,28 @@ +import type { ReactNode } from 'react'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClientProvider } from '@tanstack/react-query'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { useAllSettings } from '@/api/queries/useAllSettings'; +import { makeTestQueryClient } from '@/test/test-utils'; +import { HttpUtil, Msg } from '@/utils'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('useAllSettings', () => { + it('keeps backend-accepted settings editable when the frontend schema is stricter', async () => { + const subJsonUserAgentRegex = 'x'.repeat(2_049); + vi.spyOn(HttpUtil, 'post').mockResolvedValue(new Msg(true, '', { subJsonUserAgentRegex })); + const queryClient = makeTestQueryClient(); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + + const { result } = renderHook(() => useAllSettings(), { wrapper }); + + await waitFor(() => expect(result.current.fetched).toBe(true)); + expect(result.current.allSetting.subJsonUserAgentRegex).toBe(subJsonUserAgentRegex); + }); +}); diff --git a/frontend/src/test/zodValidate.test.ts b/frontend/src/test/zodValidate.test.ts new file mode 100644 index 000000000..44bad26dc --- /dev/null +++ b/frontend/src/test/zodValidate.test.ts @@ -0,0 +1,41 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { z } from 'zod'; + +import { HttpUtil, Msg } from '@/utils'; +import { parseMsg } from '@/utils/zodValidate'; +import { ClientPageResponseSchema } from '@/schemas/client'; +import { fetchXrayConfig } from '@/hooks/useXraySetting'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('parseMsg', () => { + it('rejects a successful response whose payload violates its schema', () => { + const msg = new Msg(true, '', { id: 'not-a-number' }); + + expect(() => parseMsg(msg, z.object({ id: z.number() }), 'test/value', { strict: true })).toThrow( + 'test/value response failed validation', + ); + }); + + it('preserves a missing successful payload for callers that handle empty values', () => { + expect(parseMsg(new Msg(true, '', null), z.object({ id: z.number() }), 'test/value').obj).toBeNull(); + }); + + it('rejects malformed paged-client payloads', () => { + const payload = { items: [], total: 'one', filtered: 1, page: 1, pageSize: 20 }; + + expect(() => parseMsg(new Msg(true, '', payload), ClientPageResponseSchema, 'clients/list/paged', { strict: true })).toThrow( + 'clients/list/paged response failed validation', + ); + }); +}); + +describe('fetchXrayConfig', () => { + it('keeps a malformed xray payload available for repair', async () => { + vi.spyOn(HttpUtil, 'post').mockResolvedValue(new Msg(true, '', JSON.stringify({ xraySetting: 'not-an-object' }))); + + await expect(fetchXrayConfig()).resolves.toEqual({ xraySetting: 'not-an-object' }); + }); +}); diff --git a/frontend/src/utils/zodValidate.ts b/frontend/src/utils/zodValidate.ts index bf614886d..2b9cd4606 100644 --- a/frontend/src/utils/zodValidate.ts +++ b/frontend/src/utils/zodValidate.ts @@ -1,10 +1,15 @@ import type { z } from 'zod'; import { Msg } from '@/utils'; +interface ParseMsgOptions { + strict?: boolean; +} + export function parseMsg( msg: Msg, schema: T, context: string, + options: ParseMsgOptions = {}, ): Msg> { if (!msg.success || msg.obj == null) { return msg as Msg>; @@ -12,6 +17,7 @@ export function parseMsg( const result = schema.safeParse(msg.obj); if (!result.success) { console.warn(`[zod] ${context} response failed validation`, result.error.issues); + if (options.strict) throw new Error(`${context} response failed validation`); return msg as Msg>; } return new Msg>(msg.success, msg.msg, result.data);