Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for completely disabling multipart request handling #66

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bindings/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Request.macro(
Request.macro('allFiles', function allFiles(this: Request) {
if (!this.__raw_files) {
throw new RuntimeException(
'Cannot read files. Make sure the bodyparser middleware is registered'
'Cannot read files. Make sure the bodyparser middleware is registered and enabled'
)
}

Expand Down
7 changes: 7 additions & 0 deletions src/bodyparser_middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,13 @@ export class BodyParserMiddleware {
if (this.#isType(ctx.request, multipartConfig.types)) {
debug('detected multipart request "%s:%s"', requestMethod, requestUrl)

if (!multipartConfig.enabled) {
throw new Exception('request content-type not supported', {
status: 415,
code: 'E_REQUEST_UNSUPPORTED_MEDIA_TYPE',
})
Comment on lines +144 to +147
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to create a custom exception class for this and expose it via an errors export, like the @adonisjs/auth package?
That way, people can use instanceof errors.... in an error handler.

However, this would require new documentation for the new exception type...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see! Thank you for clarifying!

}

ctx.request.multipart = new Multipart(ctx, {
maxFields: multipartConfig.maxFields,
limit: multipartConfig.limit,
Expand Down
1 change: 1 addition & 0 deletions src/define_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function defineConfig(config: BodyParserOptionalConfig): BodyParserConfig
},

multipart: {
enabled: true,
autoProcess: true,
processManually: [],
encoding: 'utf-8',
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export type BodyParserRawConfig = BodyParserBaseConfig
* Parser config for parsing multipart requests
*/
export type BodyParserMultipartConfig = BodyParserBaseConfig & {
enabled: boolean
autoProcess: boolean | string[]
maxFields: number
processManually: string[]
Expand Down
38 changes: 38 additions & 0 deletions tests/body_parser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,44 @@ test.group('BodyParser Middleware | form data', () => {
})
})

test('abort when multipart is not enabled', async ({ assert, cleanup }) => {
const server = createServer(async (req, res) => {
const request = new RequestFactory().merge({ req, res }).create()
const response = new ResponseFactory().merge({ req, res }).create()
const ctx = new HttpContextFactory().merge({ request, response }).create()
const middleware = new BodyParserMiddlewareFactory()
.merge({
multipart: {
enabled: false,
},
})
.create()

try {
await middleware.handle(ctx, async () => {})
} catch (error) {
res.writeHead(error.status)
res.end(error.message)
}
})
cleanup(() => {
server.close()
})

await new Promise<void>((resolve) => server.listen(3333, 'localhost', () => resolve()))

const response = await fetch('http://localhost:3333', {
method: 'POST',
headers: {
'Content-type': `multipart/form-data; boundary=9d01a3fb93deedb4d0a81389271d097f28fd67e2fcbff2932befc0458ad7`,
},
body: '--9d01a3fb93deedb4d0a81389271d097f28fd67e2fcbff2932befc0458ad7\x0d\x0aContent-Disposition: form-data; name="test"; filename="csv_files/test.csv"\x0d\x0aContent-Type: application/octet-stream\x0d\x0a\x0d\x0atest123',
})

assert.equal(response.status, 415)
assert.equal(await response.text(), 'request content-type not supported')
})

test('abort when multipart body is invalid', async ({ assert, cleanup }) => {
const server = createServer(async (req, res) => {
const request = new RequestFactory().merge({ req, res }).create()
Expand Down