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

Issue when uploading a file with multipart/form-data with Chrome but trying to upload a directory instead of a file #757

Open
JosephHalter opened this issue Sep 4, 2018 · 8 comments
Labels

Comments

@JosephHalter
Copy link

[debug] ** (Plug.Parsers.ParseError) malformed request, a MatchError exception was raised with message "no match of right hand side value: {:error, :closed}"
    (plug) lib/plug/conn.ex:1045: Plug.Conn.read_multipart_from_buffer_or_adapter/4
    (plug) lib/plug/conn.ex:936: Plug.Conn.read_part_headers/2
    (plug) lib/plug/parsers/multipart.ex:64: Plug.Parsers.MULTIPART.parse_multipart/2
    (plug) lib/plug/parsers/multipart.ex:34: Plug.Parsers.MULTIPART.parse/5
    (plug) lib/plug/parsers.ex:271: Plug.Parsers.reduce/7
    (app) lib/app_web/endpoint.ex:1: AppWeb.Endpoint.plug_builder_call/2
    (app) lib/plug/debugger.ex:122: AppWeb.Endpoint."call (overridable 3)"/2
    (app) lib/app_web/endpoint.ex:1: AppWeb.Endpoint.call/2
    (plug) lib/plug/adapters/cowboy/handler.ex:16: Plug.Adapters.Cowboy.Handler.upgrade/4
    (cowboy) /Sites/api/deps/cowboy/src/cowboy_protocol.erl:442: :cowboy_protocol.execute/4

Since plug crashes the server doesn't even return error 500, I would prefer to be able to return a proper response saying that the file seem invalid or something.

As a curl request from Chrome inspector:

curl 'http://127.0.0.1:4000/app/batch' -H 'Accept: application/vnd.api+json' -H 'Referer: http://app.test:4200/submit?batch=true' -H 'Origin: http://app.test:4200' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36' -H 'Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryUhoiSH3D5xDbVVAz' --data-binary $'------WebKitFormBoundaryUhoiSH3D5xDbVVAz\r\nContent-Disposition: form-data; name="file"; filename="Flow Chart.gstencil"\r\nContent-Type: application/octet-stream\r\n\r\n\r\n------WebKitFormBoundaryUhoiSH3D5xDbVVAz--\r\n' --compressed

I'm using Plug version 1.6.2

@josevalim
Copy link
Member

Thanks @JosephHalter! Just to make sure we can confidently reproduce the bug, can you please provide a small app (either phoenix or plug) that we can submit a request too and reproduce the failure? Thanks!

@josevalim
Copy link
Member

Ping!

@JosephHalter
Copy link
Author

JosephHalter commented Sep 23, 2018

I've created a basic app here: https://github.com/JosephHalter/demo-phoenix-app

Doing the same thing in Firefox (drag&dropping a directory instead of a file into a file input and submitting the form) creates a CURL request which is a little bit different, with a more helpful error message but it's still crashing with a 500 and not allowing me to return a 422 with a custom error message.

curl 'http://127.0.0.1:4000/app/batch' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:62.0) Gecko/20100101 Firefox/62.0' -H 'Accept: application/vnd.api+json' -H 'Accept-Language: en-US,en;q=0.5' --compressed -H 'Content-Type: multipart/form-data; boundary=---------------------------44123035693991030923554353' -H 'Connection: keep-alive' --data ''

Result:

[debug] ** (Plug.Parsers.ParseError) malformed request, a RuntimeError exception was raised with message "invalid multipart, body terminated too soon"
    (plug) lib/plug/conn.ex:1035: Plug.Conn.next_multipart/3
    (plug) lib/plug/conn.ex:950: Plug.Conn.read_part_headers/6
    (plug) lib/plug/parsers/multipart.ex:64: Plug.Parsers.MULTIPART.parse_multipart/2
    (plug) lib/plug/parsers/multipart.ex:34: Plug.Parsers.MULTIPART.parse/5
    (plug) lib/plug/parsers.ex:271: Plug.Parsers.reduce/7
    (api) lib/api_web/endpoint.ex:1: ApiWeb.Endpoint.plug_builder_call/2
    (api) lib/plug/debugger.ex:122: ApiWeb.Endpoint."call (overridable 3)"/2
    (api) lib/api_web/endpoint.ex:1: ApiWeb.Endpoint.call/2
    (plug) lib/plug/adapters/cowboy/handler.ex:16: Plug.Adapters.Cowboy.Handler.upgrade/4
    (cowboy) /api/deps/cowboy/src/cowboy_protocol.erl:442: :cowboy_protocol.execute/4

The initially reported error is still happening, however what Chrome inspector copies as cURL format doesn't match exactly what it does itself it seems as I don't get the same failure when I run it in the terminal. By capturing the request with Charles Proxy and getting the cURL request from here I get a version slightly different from Chrome:

curl -H 'Host: 127.0.0.1:4000' -H 'Accept: application/vnd.api+json' -H 'Authorization: Bearer SFMyNTY.g3QAAAACZAAEZGF0YW0AAAAkNjZkYjRmOWMtYzI0ZS00NDY0LTg4YTEtMDE5ZTNmN2UxMTE1ZAAGc2lnbmVkbgYA-u0_CGYB.PGVQWYnf8vrEJqr2yF7fyzB-RWoy48LA_7hIKh1g9xk' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36' -H 'Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryAVfsBTB6LmfDXVuV' -H 'Accept-Language: en-US,en;q=0.9,fr;q=0.8,la;q=0.7' --data-binary '------WebKitFormBoundaryAVfsBTB6LmfDXVuV
Content-Disposition: form-data; name="file"; filename="Flow Chart.gstencil"
Content-Type: application/octet-stream

' --compressed 'http://127.0.0.1:4000/api/v1/files'

Which also returns error invalid multipart, body terminated too soon instead of malformed request, a MatchError exception was raised with message "no match of right hand side value: {:error, :closed}. However Charles Proxy itself is able to replay the request with the right error message, it's just not able to give it as a cURL command, that's why I'm adding exports of the request:

request export and example of file causing the error.zip

Also in this .zip is an example of something which look like a file for MacOS but it in fact a directory which I've use to recreate the issue. I'm not sure you've got everything you need to reproduce it, but I don't know what more I can do to help.

@JosephHalter
Copy link
Author

Also, sorry for the delay I wanted to respond sooner but was on holiday. To sum up, selecting the provided "file" instead of a true file in a file input will, depending on the browser, trigger one of the following error message:

[debug] ** (Plug.Parsers.ParseError) malformed request, a MatchError exception was raised with message "no match of right hand side value: {:error, :closed}"
    (plug) lib/plug/conn.ex:1045: Plug.Conn.read_multipart_from_buffer_or_adapter/4
[debug] ** (Plug.Parsers.ParseError) malformed request, a RuntimeError exception was raised with message "invalid multipart, body terminated too soon"
    (plug) lib/plug/conn.ex:1035: Plug.Conn.next_multipart/3

or also sometimes:

[debug] ** (Plug.Parsers.ParseError) malformed request, a MatchError exception was raised with message "no match of right hand side value: {:error, :timeout}"
    (plug) lib/plug/conn.ex:1045: Plug.Conn.read_multipart_from_buffer_or_adapter/4

And what would be the best possible behaviour instead would be for the request to continue, go to the controller action with one of these 3 possibilities for the params:

  1. since the params could not be parse correctly, params = %{}
  2. the "file" value in params is blank
  3. the "file" value in params is a Plug.Upload with path linked to a file which size (from File.stat) is 0

Any of these would allow to be catched in the controller and to return a proper error message to the user such as "file is missing", "file in invalid", etc. so he can understand what is the issue.

@sebastialonso
Copy link

Was there a cause for the error, or better yet how to solve it?

@JosephHalter
Copy link
Author

@sebastialonso I don't know any way that I could better describe the problem apart from what is already in this ticket. It's not really a bug because the request IS malformed indeed. There's nothing you can do to make the request go forward, the browser tried to upload a "file" but there's no file sent. However, the issue is more about how to handle the exception, when plug crashes instead of filling something inside the connection (for example like I suggested considering that it's a file with size 0) then it'll just be an error 500 without any feedback in production.

@StevieJayCee
Copy link

StevieJayCee commented Nov 28, 2019

I was able to create this while using postman:

Create an endpoint expecting an image attribute to be provided
Create a postman test with form data and a file upload
Don't upload a file
Send the request

Though I was able to create this scenario by sending a raw POST request with no data too.

This is probably the correct response.

It's just misleading when you see the phoenix error splash screen while running phx.server in dev, vs building the release and running it in prod config where it will return a 400 status.

@JosephHalter
Copy link
Author

It's normal that the request can't proceed, but it would be better if it was possible to better catch the error. In dev when Plug.Parsers.ParseError is raised it doesn't return anything, not even 500. I didn't know that in production is returns 400. If it's the case it's already an improvement. Is there nothing else we can do to make it more user friendly? I'm not sure since it's been so long but I think that the problem is that when it crashes at the parsing level, it doesn't even go through the router.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants