> > o) provide the functionalitye of curl_formparse while:
>
> > o) providing ways of transferring data containing null characters (what
> > curl_formstore is capable of)
>
> > o) allow users to state that their input does not need to be interpreted
>
> > o) allow users to state that their buffer needs not be copied (here i am
> > not sure whether it makes sense to have this option for the name too or
> > just for the value).
>
> I chose to view it from a different angle. What kind of data does the
> function add:
>
> Basically: NAME = VALUE [ set CONTENT-TYPE ]
>
> Where each part is:
>
> NAME is a chunk of memory
>
> VALUE is one of:
> - a chunk of memory
> - one or more files
>
> CONTENT-TYPE is optionally a chunk of memory
>
> > curl_formadd (const char * name, const char * value, bool copy_value,
> > bool interpret_value, bool use_length, long value_length, struct HttpPost
> > ** firstitem, struct HttpPost ** lastitem)
>
> I don't think that this prototype would enable all the above features. The
> booleans could also be made into a single bitpattern argument.
>
> One way of solving this quite complicated set of features without using a
> huge number of arguments, could be to add a setopt()-style argument list like
> I play with below. Don't focus on the actual names, they can always be
> improved later.
>
> curl_formadd(struct HttpPost **first,
> struct HttpPost **last,
> curlform formopt,
> ...);
>
> So that a normal NAME = VALUE with copied name and data would look like:
>
> {
> struct HttpPost *first, *last;
> curl_formadd(&first, &last,
> CURLFORM_COPYNAME,
> "my_name",
> CURLFORM_COPYCONTENTS,
> "daniel",
> CURLFORM_END);
> }
>
> ... so if you want to add a form field with a copied name, but not copy the
> contents it could be like:
>
> curl_formadd(&first, &last,
> CURLFORM_COPYNAME,
> "my_name",
> CURLFORM_PTRCONTENTS, /* only point to the contents */
> "daniel",
> CURLFORM_END);
>
> ... and if you prefer to set the length of the contents instead of having
> libcurl do strlen() to get the size (which thus wouldn't work on binary
> data):
>
> curl_formadd(&first, &last,
> CURLFORM_COPYNAME,
> "my_name",
> CURLFORM_PTRCONTENTS, /* only point to the contents */
> "daniel",
> CURLFORM_SIZEOF_CONTENTS,
> 6, /* in bytes */
> CURLFORM_END);
>
> ... and a more advaned example that would add a field with two files, could
> look like:
>
> curl_formadd(&first, &last,
> CURLFORM_COPYNAME, "my_files",
> CURLFORM_FILE, "/etc/passwd",
> CURLFORM_FILE, ".login",
> CURLFORM_END);
>
> If we go with this kind of interface, I figure there's also room for a
> separate function like:
>
> curl_formaddv(struct HttpPost **first,
> struct HttpPost **last,
> va_list ap);
>
> > perhaps you have a better idea but for me personally i would rather
> > provide two calls than this overloaded one. one simple one for the most
> > common case that itself uses the more complicated one. but if you feel
> > the complicated version is enough (or you find a better way of
> > formulating the options) that is fine with me.
>
> The thing is that even if you would provide both functions you added in your
> patch, they don't provide all the features of curl_formparse(). The missing
> ability to include files or set content-type are the most obvious.

this is definitely true, i was thinking about a new function with the
proposed interface.

i think your proposal makes more sense than mine for libcurl (as my
approach is somewhat more c++ oriented, but in true c++ i would have added
default values for many variables).

i'd say i try to formulate an implemenation until the end of the week
(today and tomorrow i am too busy to start).

> > > Well, can you see any point in keeping the old 'Curl_FormReader' function?
> > > I really can't see any point with adding this extra option and complexity.
> >
> > well there is one point: my patch itself does use the old function for
> > providing the first line of the post "data" containing the mime boundary
> > string that actually is part of the http header. of course that line
> > could be obtained easily with another internal function.
> >
> > to put it in a nutshell it would be fine with me but i am not 100% sure
> > about possible consequences of changing the default behaviour.
>
> Well, it is a matter of how big chunks of data you pass to the network
> layers. It *should* not matter, so if it does, the problem should be fixed in
> the faulty code. We could of course keep the old code around until we've seen
> that the new functionality doesn't add any major trouble. I just don't want
> to add an option that all sane people want to set to TRUE, and thus all old
> programs will have to be modified to get that new functionality.

so i would say we make the rename the FastFormReader to FormReader and
insert a new internally used function for getting the first line.

> We set the 'Expect: 100-continue' header, and then we don't pass anything to
> the remote server until we've had the 100 return code properly parsed in
> lib/transfer.c. If the reply said it is a HTTP 1.1 server.
>
> I guess this is reason enough to have the CURLOPT_HTTPPOST_USE_CONTINUE
> option. As RFC2616 section 8.2.3 says:
>
> Because of the presence of older implementations, the protocol allows
> ambiguous situations in which a client may send "Expect: 100-
> continue" without receiving either a 417 (Expectation Failed) status
> or a 100 (Continue) status. Therefore, when a client sends this
> header field to an origin server (possibly via a proxy) from which it
> has never seen a 100 (Continue) status, the client SHOULD NOT wait
> for an indefinite period before sending the request body.
>
> So when posting to a HTTP 1.0 server with the Expect header, we will *have
> to* wait for a while for a possible 100-code to return, and first when we've
> decided that the response isn't coming we can continue to do the actual POST.

then i would suggest making 100-continue the default and adding some
option to allow disabling it (or use the disable-header functionality you
already mentioned).

that of course means that some old clients would like to have it turned
off and the change breaks compatibility but i would assume that people
normally should not see a difference at the application level.

as to the code already handling the 100 continue in transfer => i must
admit i did not see that piece of code. once again i will take a look at
it later on this week and try to provide a new patch.

the only thing left is to decide whether 100-continue should be the
default and how to disable or enable it.

bye,
 georg

_______________________________________________
Curl-library mailing list
http://curl.haxx.se/libcurl/
Received on 2001-08-07