On Thu, Jan 17, 2013 at 02:35:12PM +0200, Chrysovaladis Datsios wrote:
> The feature is implemented as another in curl_easy_setopt() function.
> This new option is named CURLOPT_HTTPTRAILERHEADER and adds the trailer headers
> appended on a curl_slist.
>
> Example code snippet using the new feature:
>
> struct curl_slist *trailer_http_hdrs;
> trailer_http_hdrs = curl_slist_append(trailer_http_hdrs,
> "mytrailerheader: myvalue");
> curl_easy_setopt(curl, CURLOPT_HTTPTRAILERHEADER, trailer_http_hdrs);

This interface is simple, but it obviates the entire need to have this feature
at all if such headers have to be *defined* before the transfer. Trailer
headers are designed for headers whose content isn't known, or isn't efficient
to calculate, before the transfer has been performed. What a user of this
really would want is a way to set such headers *after* the data has
been transferred.

Normally, libcurl requires all parameters of a transfer to be known and
set before the curl_easy_perform() call. In other cases where libcurl
needs application intervention during a transfer, it performs callbacks.
That would be a natural way to get the actual transfer header data immediately
before they're sent in a natural libcurl way.

But, admittedly, it would be simpler to implement a method just like
you've done, but documented in such a way that the application is allowed to
call curl_easy_setopt(CURLOPT_HTTPTRAILERHEADER,...) again from within
the data read callback once the actual header data are known. I'm not sure if
there are any other curl_easy_setopt options that are allowed to be called
during a transfer or not (I recall there may be one), which is why this
seems less desirable to me.

> ----------------------------------
> This are the diffs from the original library: curl-7.28.1
> I hope it makes it to be included in a future verson.
>
>
> diff -ur curl_orig/include/curl/curl.h curl_new/include/curl/curl.h
> --- curl_orig/include/curl/curl.h 2012-09-26 12:46:15.000000000 +0300
> +++ curl_new/include/curl/curl.h 2013-01-17 12:31:24.000460704 +0200
> @@ -1536,6 +1536,9 @@
> /* set the SMTP auth originator */
> CINIT(MAIL_AUTH, OBJECTPOINT, 217),
>
> + /* points to a linked list of trailer headers, struct curl_slist kind */
> + CINIT(HTTPTRAILERHEADER, OBJECTPOINT, 218),
> +
> CURLOPT_LASTENTRY /* the last unused */
> } CURLoption;
>
> diff -ur curl_orig/lib/http.c curl_new/lib/http.c
> --- curl_orig/lib/http.c 2012-11-13 23:02:16.000000000 +0200
> +++ curl_new/lib/http.c 2013-01-17 12:33:20.817372849 +0200
> @@ -1524,6 +1524,9 @@
> char *ptr;
> struct curl_slist *headers=conn->data->set.headers;
>
> + char *tptr;

This declaration could be moved into the while loop below, where it's needed.

> + struct curl_slist *trailer_headers=conn->data->set.trailer_headers;
> +
> while(headers) {
> ptr = strchr(headers->data, ':');
> if(ptr) {
> @@ -1590,6 +1593,31 @@
> }
> headers = headers->next;
> }
> +
> + while(trailer_headers) {
> + ptr = strchr(trailer_headers->data, ':');
> + if(ptr) {
> + tptr = --ptr; /* the point where the trailer header field ends */
> + ptr++; /* pass the colon */
> + while(*ptr && ISSPACE(*ptr))
> + ptr++;
> +
> + if(*ptr) {
> + /* only send this if the contents was non-blank */
> +
> + char tfield[CURL_MAX_HTTP_HEADER];
> + strncpy(tfield, trailer_headers->data, tptr-trailer_headers->data+1);

This will overflow tfield given a long enough user-supplied header.

> + tfield[tptr-trailer_headers->data+1] = '\0';
> + CURLcode result = Curl_add_bufferf(req_buffer, "Trailer: %s\r\n",
> + tfield);
> + tptr = NULL;

This could be moved after the next if statement.

> + if(result)
> + return result;
> + }
> + }
> + trailer_headers = trailer_headers->next;
> + }
> +
> return CURLE_OK;
> }
>
> diff -ur curl_orig/lib/transfer.c curl_new/lib/transfer.c
> --- curl_orig/lib/transfer.c 2012-11-13 23:02:16.000000000 +0200
> +++ curl_new/lib/transfer.c 2013-01-17 12:33:40.002719777 +0200
> @@ -929,6 +929,34 @@
> that instead of reading more data */
> }
>
> + /* The last chunk has zero size of data i.e. 0\r\n*/
> + if(k->upload_chunky == true && data->req.upload_present == 5 &&
> + !strncmp(data->req.upload_fromhere, "0\r\n", 3) ) {

What happens when the user wants to send the actual data "0\r\n"? This will
falsely trigger as the end of transfer. It's clearly a layering violation
performing this check way down here in the guts of the send routine.
> +
> + Curl_send_buffer *trailer_buffer = Curl_add_buffer_init();
> + result = Curl_add_bufferf(trailer_buffer, "0\r\n");
> + if(result)
> + return result;
> +
> + char *ptr;
> + struct curl_slist *trailer_headers=conn->data->set.trailer_headers;
> + while(trailer_headers) {
> + ptr = strchr(trailer_headers->data, ':');
> + if(ptr) {
> + result = Curl_add_bufferf(trailer_buffer, "%s\r\n",
> + trailer_headers->data);
> + if(result)
> + return result;
> + }
> + trailer_headers = trailer_headers->next;
> + }
> + result = Curl_add_bufferf(trailer_buffer, "\r\n");
> + if(result)
> + return result;
> + data->req.upload_fromhere = trailer_buffer->buffer;
> + data->req.upload_present = trailer_buffer->size_used;
> + }
> +
> /* write to socket (send away data) */
> result = Curl_write(conn,
> conn->writesockfd, /* socket to send to */

I assume documentation, test cases and patch to typecheck-gcc.h are still to
come :-)

>>> Dan
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-01-17