Re: Improving form post API
On 07/07/2017 11:34 AM, Daniel Stenberg wrote: I added curl_form_data_cb() to the wiki page. Seems to me this is not enough: we could have a rewind callback to support redirections and a "readdata free" callback function would be a great feature to release resources upon deletion. What about something like: |curl_form_data_cb(part, size, readfunction, rewindfunction, releasefunction, custom-pointer);| with rewindfunction and releasefunction possibly NULL ? The same rewind problem is also present for non-seekable files (i.e.: fifos and char devices): I'm afraid I have no suggestion for this. Patrick --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On 06/22/2017 09:19 AM, Patrick Monnerat via curl-library wrote: On 06/22/2017 12:20 AM, Daniel Stenberg wrote: The form API would need to become a much more generic MIME API for it to work good for that. But sure, if someone has use cases in mind and want to suggest how the current form API redesign proposal can be made into a more generic MIME one, then I could be interested. Having a mean to get a handle for each subpart could be an elegant way of doing that. I'll investigate for some proposal. What about a curl_form_subparts(part, form) function ? To support multipart MIME, it could be used as: |curl_form *form = curl_form_init(); curl_formpart *part = curl_form_addpart(form); curl_form *subform = curl_form_init(); curl_formpart *subpart = curl_form_addpart(subform); curl_form_name(subpart, "name", -1); curl_form_data(subpart, "Daniel", -1); curl_form_subparts(part, subform); | Also, the wiki note about encoding should be reinvestigated: chunked encoding has no sense for SMTP/IMAP and in this cases should rather be replaced by quoted-printable or base64. This is mainly "internal cooking", unless we want the caller to have control on it. Finally, if we decide to implement it that way, should'nt the API naming be "curl_mime_xxx" ? This has the advantage of enphasizing the difference with the old API. --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On Fri, 23 Jun 2017, Colin Hogben wrote: I've updated the wiki page after all suggestions. I also shorted the suggestion functions names somewhat by removing '_set_' from them. Overloading data-from-file and data-from-buffer in this way, using the length argument as the discriminator, seems troublesome when you want to upload an zero-length datum from a buffer. Thanks for pointing this out. Let's not overload like that if we can avoid it. At the risk of bloat, we might want a READFUNCTION/READDATA style variant for the part data too. Ah yes we do want that to be on feature-parity with the old way so we need to come up with a way to specify that. I added curl_form_data_cb() to the wiki page. -- / daniel.haxx.se --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On 6/21/2017 4:12 PM, Daniel Stenberg wrote:> How do you think we should provide a "file" in the first place? I can > probably see us having a curl_form_set_file() function for that, to be > used instead of _data() for a part. Then we could allow multiple such > function calls to add more files in the same part. Files are data as > well, sure, but they do have some more metadata that might make sense to > provide in a separate function call. One idea that comes to mind would be for curl_form_set_data() to treat file: urls as file parts and everything else as string parts. I can imagine scenarios where this would fail or produce unexpected results, though. > Hm, I was trying to make this API without using varargs arguments. I > think I > rather try separate functions. This is my chief concern. Just about any API that doesn't make use of varargs will get my stamp of approval. Aside from the problem I've mentioned with variadic functions in my binding, I think the current API is trying to do too much in a single function call and that using varargs is as much a symptom as a cause. Regards, Andrew Lambert --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On 22/06/17 22:44, Daniel Stenberg wrote: I took the liberty of updating the API suggestion in the wiki with some of the stuff I came up with in this response: https://github.com/curl/curl/wiki/formpost-API-redesigned in which: > To send "file", use curl_form_set_file instead of curl_form_set_data > > curl_form_set_name(part, "upload", -1); > curl_form_set_file(part, "/tmp/upload.tar.gz", 0, NULL); > > To instead "upload" the file from a local buffer and setting a made > up file name for it: > > curl_form_set_name(part, "upload", -1); > curl_form_set_file(part, buffer, buflen, "made-up-name"); Overloading data-from-file and data-from-buffer in this way, using the length argument as the discriminator, seems troublesome when you want to upload an zero-length datum from a buffer. I think that there are two pretty much orthogonal issues here: whether the form part is file-upload-like as seen by the server; and where the data comes from within libcurl. Perhaps we should have e.g. curl_form_set_file_info(part, filename, content_type); which populates the filename= attribute of Content-Disposition header and the Content-Type header; then curl_form_set_file_data(part, filename); for a real local file, or just use curl_form_set_data if using a buffer. Admittedly there is a little duplication of information in the case where the filename attribute is the basename of the local file. At the risk of bloat, we might want a READFUNCTION/READDATA style variant for the part data too. -- Colin Hogben --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On Thu, 22 Jun 2017, Daniel Stenberg wrote: How do you think we should provide a "file" in the first place? I can probably see us having a curl_form_set_file() function for that, to be used instead of _data() for a part. (Replying to myself) I took the liberty of updating the API suggestion in the wiki with some of the stuff I came up with in this response: https://github.com/curl/curl/wiki/formpost-API-redesigned -- / daniel.haxx.se --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On 06/22/2017 12:20 AM, Daniel Stenberg wrote: On Wed, 21 Jun 2017, Patrick Monnerat via curl-library wrote: As we may now be ready to add new entry points for form handling, I wonder if the new form API should not be extended to some more generic API, capable of building a data stream from different components for any Mime oriented protocol, reducing the need of some external Mime library for calling applications. After all, forms are a subset of Mime. This could probably be achieved without a too large "code bloat". I suppose you're then thinking of uses cases like actually providing a mail body for sending with SMTP etc? Yes, SMTP is the main target of my idea. But this could also be used for storing in IMAP. The form API would need to become a much more generic MIME API for it to work good for that. But sure, if someone has use cases in mind and want to suggest how the current form API redesign proposal can be made into a more generic MIME one, then I could be interested. Having a mean to get a handle for each subpart could be an elegant way of doing that. I'll investigate for some proposal. I just don't want it to be made into a super big and complicated project because of that... ... that's why I wrote about "code bloat". --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On Wed, 21 Jun 2017, Patrick Monnerat via curl-library wrote: As we may now be ready to add new entry points for form handling, I wonder if the new form API should not be extended to some more generic API, capable of building a data stream from different components for any Mime oriented protocol, reducing the need of some external Mime library for calling applications. After all, forms are a subset of Mime. This could probably be achieved without a too large "code bloat". I suppose you're then thinking of uses cases like actually providing a mail body for sending with SMTP etc? The form API would need to become a much more generic MIME API for it to work good for that. But sure, if someone has use cases in mind and want to suggest how the current form API redesign proposal can be made into a more generic MIME one, then I could be interested. I just don't want it to be made into a super big and complicated project because of that... -- / daniel.haxx.se --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On Tue, 20 Jun 2017, Andrew wrote: He suggests a new API where the form is an opaque handle that you get from curl_form_init, and then you call curl_form_addpart on the form handle to get a handle to a new part. Parts are populated with curl_form_set_name and curl_form_set_data. I really like the idea of using an opaque handle instead of a pointer to a list of structures, and this is basically how I already present it in my binding. Right, and that's also what we're using widely in libcurl already so that's more in line with how we usually do things. Opaque handles are way easier to support, especially long term. It also seems like encoding several CURLFORM_FILE in the same form part might be a problem if curl_form_set_data overwrites the previous data. How do you think we should provide a "file" in the first place? I can probably see us having a curl_form_set_file() function for that, to be used instead of _data() for a part. Then we could allow multiple such function calls to add more files in the same part. Files are data as well, sure, but they do have some more metadata that might make sense to provide in a separate function call. I think it might make sense to come up with 3-4 "typical" formposts and how they would be done with this new API so that we feel that it works smoothly with those. Seemingly missing in the proposal are equivalents for CURLFORM_STREAM and CURLFORM_BUFFERPTR. If curl_form_set_data were to always copy the data then I don't see how these modes could be made to work. Perhaps if instead of separate functions for the name and data there were a single curl_form_setopt function? Hm, I was trying to make this API without using varargs arguments. I think I rather try separate functions. CURLFORM_BUFFERPTR could possibly be done with the curl_form_set_file() function somehow. Perhaps as simple as: /* read a file from disk, use the file name in the part */ curl_form_set_file(part, "filename", 0, NULL); /* read a file from disk, set a custom file name in the part */ curl_form_set_file(part, "filename", 0, "curl.tar.gz"); /* pass a buffer of size 'len', call the file "monkey" */ curl_form_set_file(part, ptr, len, "monkey"); For CURLFORM_STREAM, we could maybe make curl_form_set_data() either accept an additional "flags" argument to set this or, I think less good, we could abuse the length argument to have magic values. This would also mean that you don't need separate functions for CURLFORM_CONTENTTYPE, CURLFORM_CONTENTHEADER, etc. Right, we need those too. curl_form_set_type(part, "text/html"); curl_form_set_headers(part, slist); /* standard created list */ -- / daniel.haxx.se --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
Re: Improving form post API
On 06/20/2017 10:15 PM, Andrew wrote: Daniel identifies three issues with the current API: * complicated (multitple varargs, several ways to do the same thing) * error-prone (due to the above) * the use of the public struct makes it hard to change - and yet hardly any users actually create the linked list of headers themselves! He suggests a new API where the form is an opaque handle that you get from curl_form_init, and then you call curl_form_addpart on the form handle to get a handle to a new part. Parts are populated with curl_form_set_name and curl_form_set_data. I agree with the current form API problems: several years ago, I submitted a patch to introduce a more general Mime handling API based on handles, but it has been refused at this time (redirecting me to the use of some external Mime library). Since then, libcurl environment has changed: * we have noticed the current form API limits and issues * 3 more Mime-oriented protocols (SMTP, IMAP, POP) are supported As we may now be ready to add new entry points for form handling, I wonder if the new form API should not be extended to some more generic API, capable of building a data stream from different components for any Mime oriented protocol, reducing the need of some external Mime library for calling applications. After all, forms are a subset of Mime. This could probably be achieved without a too large "code bloat". Patrick --- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
