ygrek / ocurl

OCaml bindings to libcurl
https://ygrek.org/p/ocurl
MIT License
60 stars 32 forks source link

Support CURLOPT_MIMEPOST (requires libcurl >= 7.56.0) #30

Closed nojb closed 6 years ago

nojb commented 6 years ago

Hello!

This patch adds support for CURLOPT_MIMPOST (docs here). It is quite handy to craft MIME emails, for example.

I tried to follow the existing style in curl-helper.c but you will tell me if I missed something.

Thanks!

nojb commented 6 years ago

Thanks for the review!

do you know if all setup gets duplicated on the C side by duphandle?

I think so, see https://github.com/curl/curl/blob/2b126cd7083ddf1308ebc447cabd1983b16a99fa/lib/easy.c#L893.

Please add @since 0.8.2 comment.

Done.

ygrek commented 6 years ago

I am never sure about semantics of duphandle and these lines in curl_easy_duphandle doc suggest that mime info may be shared between two handles?

All strings that the input handle has been told to point to (as opposed to copy) with previous calls to curl_easy_setopt using char * inputs, will be pointed to by the new handle as well. You must therefore make sure to keep the data around until both handles have been cleaned up.

This needs some testing to confirm..

ygrek commented 6 years ago

This is also confusing given that since 7.17.0 libcurl is copying all char* setopts. I wonder how many duphandle uses are there in the wild, esp. given that ocurl in essence does what simple closure on ocaml side would accomplish much easier and transparently..

ygrek commented 6 years ago

so indeed, the following segfaults :

  let h = init () in
  let s = String.make (1024 * 1024) 'a' in
  set_mimepost h [{encoding=CURLMIME_BINARY;headers=[];subparts=[];data=CURLMIME_DATA s}];
  let g = duphandle h in
  cleanup h;
  cleanup g
ygrek commented 6 years ago

segfault fixed in 1c54d0387f2be47a553439cd3db0ee5357fb9a7b but duphandle with CURLOPT_MIMEPOST does not work anyway, seems to be libcurl bug

nojb commented 6 years ago

Thanks for looking into this, I am a bit busy at the moment and had not found the time to do it!

ygrek commented 6 years ago

ftr libcurl bug is fixed in 7.58.0