On Sun, 5 Aug 2001, Georg Huettenegger wrote:

> so i would suggest to start the discussion about the new function right
> now: it should
>
> 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.

> if you like i can send you some implementation for the function when we
> agree on the parameters.

I would indeed appriciate that. I'm up to my ears with work on curl right now
(bugfixes, features, mails and more), so any help I can get is very welcome!

[ On the subject of the new Curl_FastFormReader ]

> > 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.

> > > o) added a 100-continue handshake for the form/data HTTP POST controlled
> >
> > Great addition. I'm not really sure we need that option though. What would be
> > the downside of _always_ sending that header when doing POST operations? (It
> > should still be able to get disabled with the regular disable-header
> > functionality.)
>
> well there are no downsides i can see. it would even be possible to not
> allow disabling it (it cannot be disabled with libwww and that seems to
> cause no problems). but if it can be disabled one needs to remember the
> fact internally to prevent transfer.c from expecting the answer.

Hm. Even if the header is used in the request, you can't assume that the 100
code will be returned. That was not the case in HTTP 1.0, and we're bound to
use such server from time to time.

> i would certainly be happy with this solution too.

I would like to see the 100-continue fix implemented slightly different. As
we already have a 100-parser in lib/transfer.c, I think it would make much
more sense if we could use that for this cause too.

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.

-- 
     Daniel Stenberg -- curl dude -- http://curl.haxx.se/
_______________________________________________
Curl-library mailing list
http://curl.haxx.se/libcurl/