Skip to content

Commit 58f385a

Browse files
slaemmerslaemmer
committed
UIFC-262 Add tests and fix ReDoS vulnerability
- Add component tests for FileUploaderField - Fix ReDoS vulnerability: replace /<[^>]*>/ regex with simple string checks for HTML tag detection (backendMessage.includes('<') && includes('>')) - Fix Sonar warnings
1 parent 15238c0 commit 58f385a

3 files changed

Lines changed: 340 additions & 11 deletions

File tree

‎src/components/Filters/FilterFile/UploadFile/FileUploaderField.js‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,16 @@ const handlePayloadTooLargeError = async (response, file, intl) => {
3636
try {
3737
const backendMessage = await response.text();
3838
// Sanitize backend message - only show if it's a simple size message
39-
// Reject messages that are too long or contain HTML tags (potential XSS)
40-
if (backendMessage && backendMessage.length < 200 && !/<[^>]*>/.test(backendMessage)) {
39+
// Reject messages that are too long or contain HTML/script tags (potential XSS)
40+
const hasHtmlTags = backendMessage.includes('<') && backendMessage.includes('>');
41+
if (backendMessage && backendMessage.length < 200 && !hasHtmlTags) {
4142
return intl.formatMessage(
4243
{ id: 'ui-finc-select.filter.file.uploadError.backendRejected' },
4344
{ message: backendMessage }
4445
);
4546
}
4647
} catch (error) {
47-
// Fall through to default message if reading response body fails
48+
console.error(error); // eslint-disable-line no-console
4849
}
4950

5051
return createFileSizeErrorMessage(file.size, intl);
@@ -76,10 +77,10 @@ const FileUploaderField = ({
7677
const handleDrop = async (acceptedFiles) => {
7778
if (acceptedFiles.length !== 1) return;
7879

79-
const file = acceptedFiles[0];
80+
const droppedFile = acceptedFiles[0];
8081

81-
if (!isFileSizeValid(file.size)) {
82-
const errorMessage = createFileSizeErrorMessage(file.size, intl);
82+
if (!isFileSizeValid(droppedFile.size)) {
83+
const errorMessage = createFileSizeErrorMessage(droppedFile.size, intl);
8384
setError(errorMessage);
8485
setIsDropZoneActive(false);
8586
setUploadInProgress(false);
@@ -91,14 +92,14 @@ const FileUploaderField = ({
9192
setUploadInProgress(true);
9293

9394
try {
94-
const response = await onUploadFile(file);
95+
const response = await onUploadFile(droppedFile);
9596

9697
if (response.ok) {
9798
const fileId = await response.text();
9899
onChange(fileId);
99100
setFile({ fileId });
100101
} else if (response.status === HTTP_STATUS_PAYLOAD_TOO_LARGE) {
101-
const errorMessage = await handlePayloadTooLargeError(response, file, intl);
102+
const errorMessage = await handlePayloadTooLargeError(response, droppedFile, intl);
102103
throw new Error(errorMessage);
103104
} else {
104105
throw new Error(

‎src/components/Filters/FilterFile/UploadFile/FileUploaderField.test.js‎

Lines changed: 323 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,323 @@
1+
import { MemoryRouter } from 'react-router-dom';
2+
import { Form, Field } from 'react-final-form';
3+
4+
import { waitFor } from '@folio/jest-config-stripes/testing-library/react';
5+
import { StripesContext, useStripes } from '@folio/stripes/core';
6+
7+
import FileUploaderField from './FileUploaderField';
8+
import { MAX_FILE_SIZE_BYTES } from '../../../../util/fileUtils';
9+
import renderWithIntlConfiguration from '../../../../../test/jest/helpers/renderWithIntlConfiguration';
10+
11+
// Mock react-dropzone to control onDrop directly
12+
let mockOnDrop;
13+
jest.mock('react-dropzone', () => ({
14+
useDropzone: (config) => {
15+
mockOnDrop = config.onDrop;
16+
return {
17+
getRootProps: () => ({ tabIndex: 0 }),
18+
getInputProps: () => ({ id: 'filter-file-input' }),
19+
};
20+
},
21+
}));
22+
23+
const mockOnUploadFile = jest.fn();
24+
const mockOnChange = jest.fn();
25+
26+
const renderComponent = (stripes, metaOverrides = {}) => {
27+
return renderWithIntlConfiguration(
28+
<StripesContext.Provider value={stripes}>
29+
<MemoryRouter>
30+
<Form
31+
onSubmit={jest.fn()}
32+
render={() => (
33+
<Field
34+
name="fileId"
35+
render={({ input, meta }) => (
36+
<FileUploaderField
37+
input={{ ...input, onChange: mockOnChange }}
38+
meta={{ ...meta, ...metaOverrides }}
39+
onUploadFile={mockOnUploadFile}
40+
/>
41+
)}
42+
/>
43+
)}
44+
/>
45+
</MemoryRouter>
46+
</StripesContext.Provider>
47+
);
48+
};
49+
50+
jest.unmock('react-intl');
51+
52+
describe('FileUploaderField', () => {
53+
let stripes;
54+
55+
beforeEach(() => {
56+
stripes = useStripes();
57+
mockOnUploadFile.mockClear();
58+
mockOnChange.mockClear();
59+
});
60+
61+
describe('Validation error timing', () => {
62+
it('should not show validation error when untouched', () => {
63+
const { container } = renderComponent(stripes, {
64+
touched: false,
65+
error: 'Required',
66+
});
67+
68+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
69+
expect(errorElement).not.toBeInTheDocument();
70+
});
71+
72+
it('should show validation error when touched', () => {
73+
const { container } = renderComponent(stripes, {
74+
touched: true,
75+
error: 'Required',
76+
});
77+
78+
const errorElement = container.querySelector('.errorMessage');
79+
expect(errorElement).not.toHaveAttribute('hidden');
80+
});
81+
82+
it('should show validation error when submit failed', () => {
83+
const { container } = renderComponent(stripes, {
84+
submitFailed: true,
85+
error: 'Required',
86+
});
87+
88+
const errorElement = container.querySelector('.errorMessage');
89+
expect(errorElement).not.toHaveAttribute('hidden');
90+
});
91+
});
92+
93+
describe('File size validation', () => {
94+
it('should accept valid files and reject oversized files', async () => {
95+
// Test 1: Accept file under limit
96+
renderComponent(stripes);
97+
98+
const smallFile = new File(['test'], 'small.txt', { type: 'text/plain' });
99+
Object.defineProperty(smallFile, 'size', { value: 1024 * 1024 }); // 1 MB
100+
101+
mockOnUploadFile.mockResolvedValue({
102+
ok: true,
103+
text: () => Promise.resolve('file-id-123'),
104+
});
105+
106+
await mockOnDrop([smallFile]);
107+
108+
expect(mockOnUploadFile).toHaveBeenCalledWith(smallFile);
109+
await waitFor(() => {
110+
expect(mockOnChange).toHaveBeenCalledWith('file-id-123');
111+
});
112+
113+
// Test 2: Accept file exactly at limit
114+
mockOnUploadFile.mockClear();
115+
mockOnChange.mockClear();
116+
117+
const maxFile = new File(['test'], 'max.txt');
118+
Object.defineProperty(maxFile, 'size', { value: MAX_FILE_SIZE_BYTES });
119+
120+
mockOnUploadFile.mockResolvedValue({
121+
ok: true,
122+
text: () => Promise.resolve('file-id-456'),
123+
});
124+
125+
await mockOnDrop([maxFile]);
126+
127+
expect(mockOnUploadFile).toHaveBeenCalledWith(maxFile);
128+
await waitFor(() => {
129+
expect(mockOnChange).toHaveBeenCalledWith('file-id-456');
130+
});
131+
});
132+
133+
it('should reject files over the limit', async () => {
134+
const { container } = renderComponent(stripes);
135+
136+
const largeFile = new File(['test'], 'large.txt');
137+
Object.defineProperty(largeFile, 'size', { value: MAX_FILE_SIZE_BYTES + 1 });
138+
139+
await mockOnDrop([largeFile]);
140+
141+
expect(mockOnUploadFile).not.toHaveBeenCalled();
142+
143+
await waitFor(() => {
144+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
145+
expect(errorElement).toBeInTheDocument();
146+
});
147+
});
148+
});
149+
150+
describe('Upload error handling', () => {
151+
it('should handle 413 with valid backend message', async () => {
152+
const { container } = renderComponent(stripes);
153+
154+
const file = new File(['test'], 'test.txt');
155+
156+
mockOnUploadFile.mockResolvedValue({
157+
ok: false,
158+
status: 413,
159+
text: () => Promise.resolve('File size limit exceeded'),
160+
});
161+
162+
await mockOnDrop([file]);
163+
164+
await waitFor(() => {
165+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
166+
expect(errorElement).toBeInTheDocument();
167+
});
168+
});
169+
170+
it('should sanitize HTML in backend messages', async () => {
171+
const { container } = renderComponent(stripes);
172+
173+
const file = new File(['test'], 'test.txt');
174+
175+
mockOnUploadFile.mockResolvedValue({
176+
ok: false,
177+
status: 413,
178+
text: () => Promise.resolve('<script>alert("xss")</script>'),
179+
});
180+
181+
await mockOnDrop([file]);
182+
183+
await waitFor(() => {
184+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
185+
expect(errorElement).toBeInTheDocument();
186+
expect(errorElement.textContent).not.toContain('<script>');
187+
});
188+
});
189+
190+
it('should reject overly long backend messages', async () => {
191+
const { container } = renderComponent(stripes);
192+
193+
const file = new File(['test'], 'test.txt');
194+
const longMessage = 'a'.repeat(250);
195+
196+
mockOnUploadFile.mockResolvedValue({
197+
ok: false,
198+
status: 413,
199+
text: () => Promise.resolve(longMessage),
200+
});
201+
202+
await mockOnDrop([file]);
203+
204+
await waitFor(() => {
205+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
206+
expect(errorElement).toBeInTheDocument();
207+
expect(errorElement.textContent).not.toContain(longMessage);
208+
});
209+
});
210+
211+
it('should handle network errors', async () => {
212+
const { container } = renderComponent(stripes);
213+
214+
const file = new File(['test'], 'test.txt');
215+
216+
mockOnUploadFile.mockResolvedValue({
217+
ok: false,
218+
status: 500,
219+
});
220+
221+
await mockOnDrop([file]);
222+
223+
await waitFor(() => {
224+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
225+
expect(errorElement).toBeInTheDocument();
226+
});
227+
});
228+
229+
it('should handle rejected promises', async () => {
230+
const { container } = renderComponent(stripes);
231+
232+
const file = new File(['test'], 'test.txt');
233+
234+
mockOnUploadFile.mockRejectedValue(new Error('Network failure'));
235+
236+
await mockOnDrop([file]);
237+
238+
await waitFor(() => {
239+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
240+
expect(errorElement).toBeInTheDocument();
241+
});
242+
});
243+
244+
it('should handle response.text() failure', async () => {
245+
const { container } = renderComponent(stripes);
246+
247+
const file = new File(['test'], 'test.txt');
248+
249+
mockOnUploadFile.mockResolvedValue({
250+
ok: false,
251+
status: 413,
252+
text: () => Promise.reject(new Error('Read failed')),
253+
});
254+
255+
await mockOnDrop([file]);
256+
257+
await waitFor(() => {
258+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
259+
expect(errorElement).toBeInTheDocument();
260+
});
261+
});
262+
});
263+
264+
describe('Edge cases', () => {
265+
it('should reject multiple files', async () => {
266+
renderComponent(stripes);
267+
268+
const file1 = new File(['test1'], 'test1.txt');
269+
const file2 = new File(['test2'], 'test2.txt');
270+
271+
await mockOnDrop([file1, file2]);
272+
273+
expect(mockOnUploadFile).not.toHaveBeenCalled();
274+
});
275+
276+
it('should handle zero-byte files', async () => {
277+
renderComponent(stripes);
278+
279+
const file = new File([''], 'empty.txt');
280+
Object.defineProperty(file, 'size', { value: 0 });
281+
282+
mockOnUploadFile.mockResolvedValue({
283+
ok: true,
284+
text: () => Promise.resolve('empty-file-id'),
285+
});
286+
287+
await mockOnDrop([file]);
288+
289+
expect(mockOnUploadFile).toHaveBeenCalledWith(file);
290+
});
291+
292+
it('should clear errors on successful upload', async () => {
293+
const { container } = renderComponent(stripes);
294+
295+
// First, trigger an error
296+
const largeFile = new File(['test'], 'large.txt');
297+
Object.defineProperty(largeFile, 'size', { value: MAX_FILE_SIZE_BYTES + 1 });
298+
299+
await mockOnDrop([largeFile]);
300+
301+
await waitFor(() => {
302+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
303+
expect(errorElement).toBeInTheDocument();
304+
});
305+
306+
// Now upload a valid file
307+
const validFile = new File(['test'], 'valid.txt');
308+
Object.defineProperty(validFile, 'size', { value: 1024 });
309+
310+
mockOnUploadFile.mockResolvedValue({
311+
ok: true,
312+
text: () => Promise.resolve('valid-file-id'),
313+
});
314+
315+
await mockOnDrop([validFile]);
316+
317+
await waitFor(() => {
318+
const errorElement = container.querySelector('.errorMessage:not([hidden])');
319+
expect(errorElement).not.toBeInTheDocument();
320+
});
321+
});
322+
});
323+
});

0 commit comments

Comments
 (0)