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

upload raises exception if Content-Type is not multipart/form-data #240

Open
cemerick opened this issue Aug 31, 2022 · 1 comment
Open
Assignees
Milestone

Comments

@cemerick
Copy link

This check is done in a couple of places:

dream/src/server/upload.ml

Lines 102 to 115 in 2386083

let content_type = match Message.header request "Content-Type" with
| Some content_type ->
Result.to_option
(Multipart_form.Content_type.of_string (content_type ^ "\r\n"))
| None ->
None
in
match content_type with
| None ->
let message =
"The request does not have 'Content-Type: multipart/form_data; ...'" in
log.error (fun log -> log "%s" message);
failwith message

The problem is that the exception that is raised is an undifferentiated Failure. This is mostly okay when handling requests from live users (as they'll just see whatever error page produced by your installed error handler/template), but is much less useful in an API context, where one would much rather produce a 400 bad request response, rather than and (erroneous) 500 internal server error.

ISTM that the upload functions that raise this failure should either return a result carrying a potential array of informative error types, or a distinct exception type be raised so that callers can issue an appropriate response.

@aantron aantron added this to the alpha6 milestone Apr 17, 2023
@aantron aantron self-assigned this Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@cemerick @aantron and others