Skip to content

Commit a45e9ff

Browse files
#952 Improve dataset endpoint error catching (#953)
* #952 Improve dataset endpoint error catching * chore: bump version to 4.3.27-20260428 [version bump] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 933a619 commit a45e9ff

5 files changed

Lines changed: 145 additions & 8 deletions

File tree

API/Backend/Datasets/routes/datasets.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,15 @@ router.post("/upload", function (req, res, next) {
299299
let working = false;
300300
let uploaded = "";
301301
let uploadFinished = false;
302-
const busboy = new Busboy({ headers: req.headers });
302+
let busboy;
303+
try {
304+
busboy = new Busboy({ headers: req.headers });
305+
} catch (err) {
306+
return res.status(400).send({
307+
status: "failure",
308+
message: "Invalid Content-Type for file upload.",
309+
});
310+
}
303311
busboy.on("file", function (fieldname, file, filename, encoding, mimetype) {
304312
file.on("data", function (data) {
305313
uploaded += data.toString("utf8");
@@ -330,6 +338,15 @@ router.post("/upload", function (req, res, next) {
330338
uploadFinished = true;
331339
populateInterval = setInterval(populateNext, 100);
332340
});
341+
busboy.on("error", function (err) {
342+
logger("error", "Busboy error during dataset upload.", req.originalUrl, req, err);
343+
if (!res.headersSent) {
344+
res.status(400).send({
345+
status: "failure",
346+
message: "Error parsing upload.",
347+
});
348+
}
349+
});
333350
req.pipe(busboy);
334351

335352
let totalPopulates = 0;

configure/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "configure",
3-
"version": "4.3.26-20260427",
3+
"version": "4.3.27-20260428",
44
"homepage": "./configure/build",
55
"private": true,
66
"dependencies": {

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "mmgis",
3-
"version": "4.3.26-20260427",
3+
"version": "4.3.27-20260428",
44
"description": "A web-based mapping and localization solution for science operation on planetary missions.",
55
"homepage": "build",
66
"repository": {

tests/e2e/api/datasets.spec.js

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { test, expect } from '@playwright/test';
2+
import { getLongTermToken, revokeLongTermToken } from '../../helpers/auth.js';
23

34
/**
45
* E2E tests for the Datasets API.
@@ -131,13 +132,88 @@ test.describe.serial('Datasets API — recreate and query lifecycle', () => {
131132
});
132133
});
133134

134-
test.describe('Datasets API — upload', () => {
135+
test.describe.serial('Datasets API — upload', () => {
135136
const baseURL = process.env.TEST_BASE_URL || 'http://localhost:18888';
137+
let longTermToken = null;
136138

137-
test('POST /api/datasets/upload — multipart CSV upload', async ({ request }) => {
138-
// Upload requires isLongTermToken (API key auth), which the test user may not have.
139-
// We still validate the endpoint doesn't crash.
140-
test.skip(true, 'SKIP: Dataset upload requires long-term API token (isLongTermToken) — not available in standard test auth');
139+
test.beforeAll(async ({ request }) => {
140+
longTermToken = await getLongTermToken(request);
141+
});
142+
143+
test.afterAll(async ({ request }) => {
144+
if (longTermToken) await revokeLongTermToken(request, longTermToken);
145+
});
146+
147+
// --- Crash regression tests (busboy error handling) ---
148+
149+
test('POST /api/datasets/upload — invalid Content-Type does not crash server (returns 400, not 500)', async ({ request }) => {
150+
// Sending a non-multipart Content-Type previously caused busboy to throw
151+
// synchronously with no try/catch, crashing the process. After the fix it
152+
// must return 400 (when a valid token is present) or a non-5xx auth failure.
153+
const response = await request.post(`${baseURL}/api/datasets/upload`, {
154+
headers: {
155+
...(longTermToken ? { Authorization: `Bearer ${longTermToken}` } : {}),
156+
'Content-Type': 'text/plain',
157+
},
158+
data: 'not a multipart body',
159+
});
160+
expect(response.status()).not.toBe(500);
161+
if (longTermToken) {
162+
expect(response.status()).toBe(400);
163+
const body = await response.json();
164+
expect(body.status).toBe('failure');
165+
}
166+
});
167+
168+
test('POST /api/datasets/upload — missing Content-Type does not crash server', async ({ request }) => {
169+
// Busboy throws 'Missing Content-Type' synchronously when the header is absent.
170+
const response = await request.post(`${baseURL}/api/datasets/upload`, {
171+
headers: {
172+
...(longTermToken ? { Authorization: `Bearer ${longTermToken}` } : {}),
173+
'Content-Type': '',
174+
},
175+
data: '',
176+
});
177+
expect(response.status()).not.toBe(500);
178+
});
179+
180+
// --- Auth gate ---
181+
182+
test('POST /api/datasets/upload — without token returns failure, not 5xx', async ({ request }) => {
183+
const response = await request.post(`${baseURL}/api/datasets/upload`, {
184+
multipart: {
185+
name: `upload_noauth_${Date.now()}`,
186+
header: JSON.stringify(['name', 'value']),
187+
upsert: 'false',
188+
file: { name: 'data.csv', mimeType: 'text/csv', buffer: Buffer.from('name,value\nfoo,1\n') },
189+
},
190+
});
191+
expect(response.status()).not.toBe(500);
192+
const body = await response.json();
193+
expect(body.status).toBe('failure');
194+
});
195+
196+
// --- Happy-path upload (only when a long-term token was obtained) ---
197+
198+
test('POST /api/datasets/upload — valid multipart CSV upload does not return 5xx', async ({ request }) => {
199+
if (!longTermToken) {
200+
test.skip(true, 'SKIP: Could not obtain a long-term token');
201+
return;
202+
}
203+
204+
const csvContent = ['name,score', 'alpha,10', 'beta,20', 'gamma,30'].join('\n') + '\n';
205+
const response = await request.post(`${baseURL}/api/datasets/upload`, {
206+
headers: { Authorization: `Bearer ${longTermToken}` },
207+
multipart: {
208+
name: `upload_e2e_${Date.now()}`,
209+
header: JSON.stringify(['name', 'score']),
210+
upsert: 'false',
211+
file: { name: 'data.csv', mimeType: 'text/csv', buffer: Buffer.from(csvContent) },
212+
},
213+
});
214+
expect(response.status()).not.toBe(500);
215+
const body = await response.json();
216+
expect(body).toHaveProperty('status');
141217
});
142218
});
143219

tests/helpers/auth.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,47 @@ export async function loginAsAdmin(request) {
9191
password: DEFAULT_ADMIN.password,
9292
});
9393
}
94+
95+
/**
96+
* Generate a short-lived long-term token via the admin API.
97+
*
98+
* Requires the server to be running with AUTH enabled and the test admin
99+
* account to have admin privileges. Returns `null` if the token could not
100+
* be obtained (e.g. AUTH=off, no admin account).
101+
*
102+
* Callers are responsible for revoking the token when done (see
103+
* `revokeLongTermToken`).
104+
*
105+
* @param {import('@playwright/test').APIRequestContext} request
106+
* @param {string} [name] - Human-readable label for the token.
107+
* @returns {Promise<string|null>} The raw token string, or null.
108+
*/
109+
export async function getLongTermToken(request, name = `test_token_${Date.now()}`) {
110+
const resp = await request.post('/api/longtermtoken/generate', {
111+
data: { name, period: '1d' },
112+
});
113+
if (resp.status() !== 200) return null;
114+
const body = await resp.json();
115+
if (body.status !== 'success' || !body.body?.token) return null;
116+
return body.body.token;
117+
}
118+
119+
/**
120+
* Revoke a long-term token by its token string.
121+
*
122+
* Looks up the token in the listing and deletes it. Silently no-ops if
123+
* the token is not found or the listing call fails.
124+
*
125+
* @param {import('@playwright/test').APIRequestContext} request
126+
* @param {string} token - The token string returned by `getLongTermToken`.
127+
* @returns {Promise<void>}
128+
*/
129+
export async function revokeLongTermToken(request, token) {
130+
const listResp = await request.get('/api/longtermtoken/get');
131+
if (listResp.status() !== 200) return;
132+
const listBody = await listResp.json();
133+
if (listBody.status !== 'success') return;
134+
const entry = listBody.tokens?.find((t) => t.token === token);
135+
if (!entry) return;
136+
await request.post('/api/longtermtoken/clear', { data: { id: entry.id } });
137+
}

0 commit comments

Comments
 (0)