On Thu, 16 May 2002, J. Cone wrote:

> Please find two attachments, one showing the error on test 1, and the other
> containing the next revision of the patch.

The failure of test 1 indicates a problem with the patch. For some reason it
includes an authorization in the request, even when not told to. This is a
serious flaw. This problem seems to be because of this piece of code:

+ /* If our protocol needs a password and we have none, use the defaults */
+ if ( (conn->protocol & (PROT_FTP|PROT_HTTP)) &&
+ !conn->bits.user_passwd) {
+ strcpy(data->state.user, CURL_DEFAULT_USER);
+ strcpy(data->state.passwd, CURL_DEFAULT_PASSWORD);
+ conn->bits.user_passwd = 1; /* enable user+password */
   }

You unconditionally set the user_passwd TRUE here because of the
unconditional setting it to FALSE first (more elaborataton on this follow
below). That is badness.

I like the way you solved the ~/.netrc-testing in the test suite problem. I
might change the dependency on the MALLOCDEBUG symbol though, as it looks
just too weird. I think we should either introduce a more generic DEVELDEBUG
or add a separate NETRCDEBUG. I think I'd prefer the former.

My comments on details of the patch:

+ /* We may have a user name and password, but they're not there yet. */
+ conn->bits.user_passwd = FALSE;

   /* inherite initial knowledge from the data struct */
- conn->bits.user_passwd = data->set.userpwd?1:0;

Why did you change this? This looks like an alteration of behavior. If the
user and password is set in the SessionHandle struct, that status needs to be
passed to this connection handle. I can't se how you can assume that it isn't
there even when told so.

Or am I missing the clever part further down in the code?

- else {
- /* traditional IPv4-style port-extracting */
- tmp = strchr(conn->name, ':');
- }
+ tmp = strrchr(conn->name, ':');
+
+ if (tmp &&
+ strlen(tmp+1) == strspn(tmp+1, "0123456789")) {
+ /* The last colon really was part of the port number */

This fix makes the code more complex with no extra functionality, AFAICS (it
also adds a dependency on strspn()). As we already discussed, there can be
only one colon at that point and if there is no digits on the right side of
it, well then it is badly written URL in the first place. I suggest you leave
the previous colon-finding and port number extraction code.

After all, it has worked fine like that for quite some time! ;-)

-- 
    Daniel Stenberg -- curl groks URLs -- http://curl.haxx.se/
_______________________________________________________________
Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: bandwidth_at_sourceforge.net