Skip to content

Commit

Permalink
Avoid non-blob webresources fields on multipart requests
Browse files Browse the repository at this point in the history
Change-type: patch
  • Loading branch information
otaviojacobi committed Apr 12, 2024
1 parent 6b58f09 commit a5f6022
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
34 changes: 25 additions & 9 deletions src/webresource-handler/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const getWebresourceHandler = (): WebResourceHandler | undefined => {
const isFileInValidPath = async (
fieldname: string,
req: Express.Request,
odataRequest: uriParser.ParsedODataRequest,
): Promise<boolean> => {
if (req.method !== 'POST' && req.method !== 'PATCH') {
return false;
Expand All @@ -77,10 +78,6 @@ const isFileInValidPath = async (
return false;
}
const model = getModel(apiRoot);
const odataRequest = uriParser.parseOData({
url: req.url,
method: req.method,
});
const sqlResourceName = sbvrUtils.resolveSynonym(odataRequest);

const table = model.abstractSql.tables[sqlResourceName];
Expand Down Expand Up @@ -124,6 +121,15 @@ export const getUploaderMiddlware = (
const bb = busboy({ headers: req.headers });
let isAborting = false;

const parsedOdataRequest = uriParser.parseOData({
url: req.url,
method: req.method,
});
const webResourcesFieldNames = getWebResourceFields(
parsedOdataRequest,
false,
);

const finishFileUpload = () => {
req.unpipe(bb);
req.on('readable', req.read.bind(req));
Expand Down Expand Up @@ -151,7 +157,9 @@ export const getUploaderMiddlware = (
completeUploads.push(
(async () => {
try {
if (!(await isFileInValidPath(fieldname, req))) {
if (
!(await isFileInValidPath(fieldname, req, parsedOdataRequest))
) {
filestream.resume();
return;
}
Expand Down Expand Up @@ -182,7 +190,15 @@ export const getUploaderMiddlware = (
// multipart requests will have two main parts, the file contents and the form fields
// This receives the form fields and transforms them into a standard JSON body
// This is a similar behavior as previous multer library did
bb.on('field', (name, val) => {
bb.on('field', async (name, val) => {
if (webResourcesFieldNames.includes(name)) {
isAborting = true;
bb.emit(
'error',
new errors.BadRequestError('WebResource field must be a blob.'),
);
return;
}
req.body[name] = val;
});

Expand All @@ -207,17 +223,17 @@ export const getUploaderMiddlware = (
}
});

bb.on('error', async (err) => {
bb.on('error', async (err: Error) => {
await clearFiles();
finishFileUpload();
next(err);
sbvrUtils.handleHttpErrors(req, res, err);
});
req.pipe(bb);
};
};

const getWebResourceFields = (
request: uriParser.ODataRequest,
request: uriParser.ODataRequest | uriParser.ParsedODataRequest,
useTranslations = true,
): string[] => {
// Translations will use modifyFields(translated) rather than fields(original) so we need to
Expand Down
10 changes: 10 additions & 0 deletions test/06-webresource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,16 @@ describe('06 webresources tests', function () {
expect(await isEventuallyDeleted(uniqueFilename)).to.be.true;
});

it('should not accept webresource payload that is not a blob', async () => {
const res = await supertest(testLocalServer)
.post(`/${resourceName}/organization`)
.field('name', 'John')
.field(resourcePath, 'not a blob')
.expect(400);

expect(res.body).to.equal('WebResource field must be a blob.');
});

it('should not accept webresource payload on application/json requests', async () => {
const uniqueFilename = `${randomUUID()}_${filename}`;

Expand Down

0 comments on commit a5f6022

Please sign in to comment.