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