search

benoitc / hackney

simple HTTP client in Erlang
Other
1.33k stars 427 forks source link

Value of list-type gets used as "Len"(integer) when making {multipart [{K,V}]} request body #171

Open ubill opened 9 years ago

ubill commented 9 years ago

I am trying to POST a request body of multipart.

I am getting this error at hackney_multipart.erl , line 181:

\ exception error: an error occurred when evaluating an arithmetic expression in function hackney_multipart:'-len_mp_stream/2-fun-0-'/3 (src/hackney_lib/hackney_multipart.erl, line 181) in call from lists:foldl/3 (lists.erl, line 1261) in call from hackney_multipart:len_mp_stream/2 (src/hackney_lib/hackney_multipart.erl, line 159) in call from hackney_request:handle_body/4 (src/hackney_client/hackney_request.erl, line 306) in call from hackney_request:perform/2 (src/hackney_client/hackney_request.erl, line 76) in call from hackney:send_request/2 (src/hackney_client/hackney.erl, line 347)

How to reproduce this, do this: At = "AFSGVASFAV..." % type 'list' Url2 = "some valid url.." ReqHeaders = [], Body = {multipart, [{<<"access_token">>,At}]}, {ok, Code, Headers, Client} = hackney:request(post, Url2, ReqHeaders, Body)

This problem is 'solved' by wrapping the At in list_to_binary: Body = {multipart, [{<<"access_token">>,list_to_binary(At)}]},

The root cause of the problem seems to be when I use a KV pair inside multipart and my Value is a 'list', the value of Len in like 181 takes the value of the list instead of its length(integer) thus an arithmetic error is thrown.

ln 181: AccSize + byte_size(MpHeader) + Len + 2;

            ({Name, Len}, AccSize) ->
                {MpHeader, Len} = mp_data_header({Name, Len}, Boundary),

                io:format("==============================~n"),
                io:format("~p ~p~n", [AccSize, MpHeader]),
                io:format("~p~n", [Len]),
                io:format("~p~n", [byte_size(MpHeader)]),
                io:format("==============================END~n"),

                AccSize + byte_size(MpHeader) + Len + 2;

Is this a valid bug? Besides that, I do have some (maybe newbie) questions: 1 - Shouldn't the {K,V} "just work" regardless of whether the K and V are atoms, lists or binaries? At least that is what I am led to believe by reading the Hackney documentation. 2 - Besides that, how do I use hackney to POST a form with some string post params, and a file upload param? Here's what I want to do, and I discovered the above issue when trying to do this using Hackney:

// This curl command line works curl -F 'access_token=CAACEdEose' -F 'source=@cat.jpg' -F 'message=220px cat from wikipedia' https://graph.facebook.com/12345678/photos

whereby the file cat.jpg is uploaded in the post field named "source", and other parameters are named "access_token" and "message" and they have string values. How to do this in Hackney?

benoitc commented 9 years ago

It is expected right now that K/V in the multipart are binaries. I recently added some guards to return a more understandable error in b018bed040d830c5d4232e43d46806191da01e42 (latest master).

For the second part of your question can you paste me the full request done by curl in verbose mode? I will check if it's feasible.

ubill commented 9 years ago

Hi benoitc wow, thanks for the quick reply!

Here is the full verbose output of curl and the command line used to get it. (I have snipped out those access tokens and the like though.)

And also, in case you need to see it, this usage is from the action "upload photo" as described at this page in the Facebook API docs: https://developers.facebook.com/docs/graph-api/reference/v2.2/album/photos under """Publishing: There are two separate ways of publishing photos to Facebook: Capture a photo via file upload as multipart/form-data then use the source parameter: ... """

curl -i -v -F 'access_token=XXXXXX' -F 'source=@cat.jpg' -F 'message=220px cat from wikipedia' https://graph.facebook.com/12345678/photos

<

benoitc commented 9 years ago

So apparently curl is doing a multipart/form-data. It;' not handled automatically yet in hackney but you can send such multipart request manually. Makes sure that a field is a part in that case. I will see if i cann the support for it in next release or maybe the one after since I am mostly focusing on bug fixes and monitoring/logging right now.

ubill commented 9 years ago

Thank you for your effort benoitc!

"""you can send such multipart request manually.""" I am right now trying hard to do this - but I just can't figure out though the syntax that I must use in order to do this. Here's what I have now:

Parts = [ {<<"access_token">>, list_to_binary(At)}, {<<"message">>, <<"ErlangPix Posted into album, ensure 'message' has no illegal characters...">>},

    {file, <<"cat.jpg">>}  % I don't know what to write here.
],

ReqHeaders = [], Body = {multipart, Parts}, {ok, Code, Headers, Client} = hackney:request(post, Url2, ReqHeaders, Body)

benoitc commented 9 years ago

Yes the file will be streamed in that case using sendfile. You can also use hackney:stream_multipart/1 and stream each part until EOF.

benoitc commented 9 years ago

anything to add to this ticket?

ubill commented 9 years ago

(Yes, I am getting together the details)

ubill commented 9 years ago

Right now, with curl, we can make a POST request of the form like ... -F 'source=@cat.jpg' -F 'message=220px cat from wikipedia'

which posts to a form with input fields of name 'message' (type="text") and 'source' (type="file")

Note that we can specify the "name" of the file input field.

First name: [ input type="text" name="message" ] Select a file: [ input type="file" name="source" ]

1) Using hackney, making the POST request with ExtraHeaders = [] and tracing the intermediate composed output

CODE: hackney_request.erl: %% internal handle_body(Headers, ReqType0, Body0, Client) -> ... ... {NewHeaders, ReqType, Body, Client}.

printing "Body": {

Fun,

[<<"-----------------------------mtynipxrmpegseog\r\ncontent-length: 204\r\ncontent-type: application/octet-stream\r\ncontent-disposition: form-data; name=\"access_token\"\r\n\r\nCAAJI5JaPJBIVvsZD\r\n">>,
   <<"-----------------------------mtynipxrmpegseog\r\ncontent-length: 74\r\ncontent-type: application/octet-stream\r\ncontent-disposition: form-data; name=\"message\"\r\n\r\nErlangPix Posted into album, ensure 'message' has no illegal characters...\r\n">>,
   <<"-----------------------------mtynipxrmpegseog\r\ncontent-length: 181441\r\ncontent-type: image/jpeg\r\ncontent-disposition: form-data; name=file; filename=\"cat.jpg\"\r\n\r\n">>,
   {file,<<"cat.jpg">>},
   <<"\r\n">>,<<"-----------------------------mtynipxrmpegseog--">>]

},

will result in output of content-disposition: form-data; name=file; filename=\"cat.jpg\" and {file,<<"cat.jpg">>} to be sent out by hackney.

Q1. It appears that we don't get to specify the "name" of the file input field that we want?

2) Calling the same POST function, but now with Parts and ExtraHeaders as below,

ExtraHeaders = [{<<"name">>, <<"source">>}],

Parts = [ {<<"access_token">>, list_to_binary(At)}, {<<"message">>, <<"ErlangPix Posted into album, ensure 'message' has no illegal characters...">>}, {file, <<"cat.jpg">>, ExtraHeaders} ],

printing "Body": {#Fun, [<<"-----------------------------mtynipxrmpegseog\r\ncontent-length: 204\r\ncontent-type: application/octet-stream\r\ncontent-disposition: form-data; name=\"access_token\"\r\n\r\nCAAJI5JaPJBIVvsZD\r\n">>, <<"-----------------------------mtynipxrmpegseog\r\ncontent-length: 74\r\ncontent-type: application/octet-stream\r\ncontent-disposition: form-data; name=\"message\"\r\n\r\nErlangPix Posted into album, ensure 'message' has no illegal characters...\r\n">>, <<"-----------------------------mtynipxrmpegseog\r\ncontent-length: 181441\r\ncontent-type: image/jpeg\r\ncontent-disposition: form-data; name=file; filename=\"cat.jpg\"\r\nname: source\r\n\r\n">>, {file,<<"cat.jpg">>}, <<"\r\n">>,<<"-----------------------------mtynipxrmpegseog--">>]}

we get content-disposition: form-data; name=file; filename=\"cat.jpg\"\r\nname: source and {file,<<"cat.jpg">>}

Q2. Now my question is, how do we specify the "name" of the file input field of our form, as curl allows us to do?

In my case, both the above methods happened to work for me (so I went with the ExtraHeaders = [] in the end), but I was wondering if there are forms which need the specific "name" for their file input field to be correct and that I would need to correctly specify the form field "name" for my file upload.

Q3. Is that possible in Hackney? How would I do that? Thanks.

benoitc commented 9 years ago

You can indeed use {file, Path, ExtraHeaders} to add the name. Now I guess we could add another pattern {file, Name, Path, ExtraHeaders} to do it. Would that works for you?

ubill commented 9 years ago
seantanly commented 9 years ago

:+1:

Any chance {file, Name, Path, ExtraHeaders} will be implemented and released soon? Not being able to name the file input field name seems like a missing critical feature.

Meanwhile, I have tried using ExtraHeaders with [{<<"name">>, <<"source">>}], but can't that to define the file input name. Is there a workaround for this?

benoitc commented 9 years ago

@seantanly Not sure if there will be another option yet. Anyway, you can change the name by setting the disposition header:

https://github.com/benoitc/hackney/blob/master/src/hackney_lib/hackney_multipart.erl#L232

Something like

Disposition = {<<"form-data">>,
                       [{<<"name">>, <<"file">>},
                        {<<"filename">>, <<"\"", FName/binary, "\"">

And then use {file, Path, Disposition, ExtraHeaders} . Hope it helps,

seantanly commented 9 years ago

Cool, that works, thanks for the quick reply!

Nevertheless, I do hope the custom input name support will be added, as it is quite a common use case .

Also, looking at the README#Send a body, there is no mention of the {file, Path, Disposition, ExtraHeaders} option. It seems to be a mouthful to explain to new users how to achieve this with the code example above, so perhaps it's better to officially support the use case as an option.