Hello Daniel,

I'm not sure that my mailer does quoting correctly, so I have divided your
replies into sections and reply to them here.

AAA && CCC:

I think I don't understand what conn->bits.user_passwd being true means. I
have been assuming it means only that data->state.user and
data->state.passwd are defined.

Please explain how it can be that:
    we are doing FTP or HTTP
and
    data->state.user and data->state.passwd are defined
and
   conn->bits.user_passwd is false?

Does this happen when the user and password are the defaults? Not setting
the flag in that case makes the tests complete.

I think I have introduced a bug we haven't discovered yet; if all of the
outputs were set in the section at line 1865, then the section at line 1948
that interprets data->set.userpwd should be setting conn->bits.user_passwd
and isn't. I've changed the test at line 1314 back, to minimise code
change. If you think interpreting data->set.userpwd once only is a good
idea, then I could assign it false at line 1314, as I initially wanted, and
true at about line 1948, when I discover that data->set.userpwd is set and
use the contents.

BBB:

Thank you; I think it was your idea originally. I've fixed it some more,
so it maintains $skipped.

There is a _NETRC_DEBUG that causes netrc.c to make a main(). This is not
controlled by --enable-debug in the configure script. It's different from
the DEVELDEBUG that you're considering, isn't it?

Please can renaming MALLOC_DEBUG be a separate patch, to keep down the
number of changed files in my patch?

DDD:

When you say that there is only one colon in the URL, I disagree. I have
moved the code that extracts the username and password further down the
file, so that it won't be confused by colons in port numbers or IPV6
addresses. That means that here, there could be colons from:
   - username:password
   - IPv6 address
   - port number

If you consider the whole of the change, up to the block comment, it saves
nine non-blank lines, and makes IPv6 addresses not need a special case
(although I admit to not having tested them - is there someone who could
with relatively small grief?).

We could get the same effect as strspn (testing that everything after the
colon is digits, so we can protect ourselves against a false colon earlier
in the string) by using strtoul, if you prefer that over strspn.

I *really* don't want to add a special case to the IPv4 case, for finding
the possible "@" and then the possible colon and then blindly doing atoi on
something I haven't tested is a digit string. Am I making sense here?

No patch yet, because AAA, CCC and DDD are design conversations that affect
what goes into it.

Regards,
James C.

At 13:37 16/05/2002 +0200, Daniel Stenberg wrote:
>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.

AAA:

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

BBB:

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

CCC:

>+ /* 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?

DDD:

>- 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
Received on 2002-05-17