hi daniel,

> > o) the new default behaviour of the library is to use a 100-continue
> > handshake with the webserver. if this is not appropriate disable the
> > header (using curl_easy_setopt (CURLOPT_HTTPHEADER)) - this is shown in
> > the new sample postit2.c This change affects urldata.h, url.c, http.c,
> > and transfer.c (that waits for the 100-continue header before it writes
> > anything - if a timeout is encountered it nevertheless writes without
> > waiting any further).
>
> This is 99% fine. I can only think of this minor detail we still need to fix:
> when we do a POST with the Expect: header present. What if the server returns
> "417 Expectation Failed" ? It should make us _not_ send a single byte of the
> post stream. And similarly, if we have timed-out waiting for 100 and thus
> started to send data, we can still get "417 Expectation Failed" and thus we
> should stop sending POST data. Neither of these cases happen right now.

that is true, do you mean i shoudl take a look to achieve these goals (did
not think about them as i am not that firm when it comes to http posting).

> BTW, I just added test case 44 that tests posting without Expect: header,
> which seems to work just fine.

very good.

> > it does allow the following: adding simple name/content sections, adding
> > single and multiple file sections, adding sections where a pointer to the
> > memory is provided (and in case the length is also provided the memory is
> > allowed to contain null characters). Everything may have a contenttype.
>
> I like it. I do have some suggestions though, that'll make the function more
> robust and less error-prone when used:
>
> o store all input data in one struct and act on all the data when the
> CURLFORM_END is received, it makes the order of the input totally
> irrelevant and it proably also would make the code smaller. Hm, we still
> need to support multiple CURLFORM_FILE items of course and thus we must
> probly enforce a strict order in which the CURLFORM_FILE must be set
> before CURLFORM_CONTENTTYPE.
>
> o CURLFORM_COPYCONTENTS should support that we specify the length too,
> and not require strlen() to work. The CURLFORM_CONTENTSLENGTH would work
> the same way for both CURLFORM_COPYCONTENTS and CURLFORM_PTRCONTENTS.
>
> o The exact same would go for the name, which then could be copied or
> pointed to, which an optional length.
>
> Or am I wrong?

well i agree with most of your points but i still do not see the benefit
of allowing unordered input. the code already allows some variants (like
having the length before the type and vice versa) but there will always be
two parts: the part with the name and the content part that additionally
may consist of multiple file parts.

i would propose to add PTRNAME and to allow CONTENTSLENGTH for COPYNAME,
PTRNAME, and COPYCONTENTS and (as should be possible now) to allow for
a different ordering of the CONTENTTYPE and the CONTENTLENGTH.

as for code reduction i would rather try to put some code into a seperate
function that to try to handle everything at once. so i would see a
function that does parse the CONTENTTYPE and CONTENTSLENGTH as a possible
way of getting maintainable code.

what do you think, should i try to enhance curl_formadd as outlined by me?

> Feel free to get the lot from CVS now and have a look!

great, will take a look later on this week (also changes from me would
propably not arrive before sunday/monday).

bye,
 georg
Received on 2001-08-21