Skip to content

Commit 472db74

Browse files
authored
Reduce backend data leakage on frontend (#211)
* Log mimimum data for client-side to function for file upload * Fix buildUploadStatusUrl so it works without a prefix * restore missing test * Test correct output for file upload status API
1 parent e53487e commit 472db74

4 files changed

Lines changed: 72 additions & 14 deletions

File tree

src/client/javascripts/file-upload.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,13 +230,27 @@ function reloadPage() {
230230

231231
/**
232232
* Build the upload status URL given the current pathname and the upload ID.
233+
* This only works when called on a file upload page that has a maximum depth of 1 URL segments following the slug.
233234
* @param {string} pathname – e.g. window.location.pathname
234235
* @param {string} uploadId
235236
* @returns {string} e.g. "/form/upload-status/abc123"
236237
*/
237238
export function buildUploadStatusUrl(pathname, uploadId) {
238-
const pathSegments = pathname.split('/').filter((segment) => segment)
239-
const prefix = pathSegments.length > 0 ? `/${pathSegments[0]}` : ''
239+
// Remove preview markers and duplicate slashes
240+
const normalisedPath = pathname
241+
.replace(/\/preview\/(draft|live)/g, '')
242+
.replace(/\/{2,}/g, '/')
243+
.replace(/\/$/, '')
244+
245+
const segments = normalisedPath.split('/').filter(Boolean)
246+
247+
// The slug is always the second to last segment
248+
// The prefix is everything before the slug
249+
const prefix =
250+
segments.length > 2
251+
? `/${segments.slice(0, segments.length - 2).join('/')}`
252+
: ''
253+
240254
return `${prefix}/upload-status/${uploadId}`
241255
}
242256

src/server/index.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,9 @@ describe('Upload status route', () => {
508508
const res = await server.inject(options)
509509

510510
expect(res.statusCode).toBe(StatusCodes.OK)
511-
expect(res.result).toEqual(mockStatus)
511+
expect(res.result).toEqual({
512+
uploadStatus: UploadStatus.ready
513+
})
512514
expect(getUploadStatus).toHaveBeenCalledWith(
513515
'123e4567-e89b-12d3-a456-426614174000'
514516
)

src/server/plugins/engine/routes/file-upload.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ export async function getHandler(
2222
return h.response({ error: 'Status check failed' }).code(400)
2323
}
2424

25-
return h.response(status)
25+
return h.response({
26+
uploadStatus: status.uploadStatus
27+
})
2628
} catch (err) {
2729
request.logger.error(
2830
err,

test/client/javascripts/file-upload.test.js

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,21 +1345,61 @@ describe('File Upload Client JS', () => {
13451345
})
13461346
})
13471347

1348-
describe('buildUploadStatusUrl()', () => {
1349-
it('builds URL with no prefix for root paths', () => {
1350-
expect(buildUploadStatusUrl('/', 'abc')).toBe('/upload-status/abc')
1351-
expect(buildUploadStatusUrl('', 'xyz')).toBe('/upload-status/xyz')
1348+
describe('buildUploadStatusUrl', () => {
1349+
it('works with no prefix', () => {
1350+
expect(buildUploadStatusUrl('/my-form/file-upload-page', '123')).toBe(
1351+
'/upload-status/123'
1352+
)
1353+
1354+
expect(
1355+
buildUploadStatusUrl('/preview/draft/my-form/file-upload-page', '123')
1356+
).toBe('/upload-status/123')
1357+
1358+
expect(
1359+
buildUploadStatusUrl('/preview/live/my-form/file-upload-page', '123')
1360+
).toBe('/upload-status/123')
13521361
})
13531362

1354-
it('uses the first segment as prefix', () => {
1355-
expect(buildUploadStatusUrl('/form/mypage', 'id1')).toBe(
1356-
'/form/upload-status/id1'
1363+
it('works with a prefix with a single level', () => {
1364+
expect(buildUploadStatusUrl('/form/my-form/file-upload-page', '123')).toBe(
1365+
'/form/upload-status/123'
13571366
)
1367+
1368+
expect(
1369+
buildUploadStatusUrl(
1370+
'/form/preview/draft/my-form/file-upload-page',
1371+
'123'
1372+
)
1373+
).toBe('/form/upload-status/123')
1374+
1375+
expect(
1376+
buildUploadStatusUrl('/form/preview/live/my-form/file-upload-page', '123')
1377+
).toBe('/form/upload-status/123')
1378+
})
1379+
1380+
it('works with a prefix with multiple levels', () => {
1381+
expect(
1382+
buildUploadStatusUrl('/org/form/my-form/file-upload-page', '123')
1383+
).toBe('/org/form/upload-status/123')
1384+
1385+
expect(
1386+
buildUploadStatusUrl(
1387+
'/org/form/preview/draft/my-form/file-upload-page',
1388+
'123'
1389+
)
1390+
).toBe('/org/form/upload-status/123')
1391+
1392+
expect(
1393+
buildUploadStatusUrl(
1394+
'/org/form/preview/live/my-form/file-upload-page',
1395+
'123'
1396+
)
1397+
).toBe('/org/form/upload-status/123')
13581398
})
13591399

1360-
it('trims nested segments and trailing slashes', () => {
1361-
expect(buildUploadStatusUrl('/one/two/three/', 'id2')).toBe(
1362-
'/one/upload-status/id2'
1400+
it('trims trailing slashes', () => {
1401+
expect(buildUploadStatusUrl('/my-form/file-upload-page/', '123')).toBe(
1402+
'/upload-status/123'
13631403
)
13641404
})
13651405
})

0 commit comments

Comments
 (0)