On Wed, 3 Apr 2002, J. Cone wrote:

> Please find enclosed a patch which:
> - is released under the MIT licence with the permission of Ms K Couper
> at eServGlobal in Wellington
> - is against curl 7.9.5

*thanks* (any particular reason why Ms K Couper isn't posting this himself?)

I appreciate the amount of comments in the patch and its general usability,
but I have a few remarks on the patch, as follows here. Please don't take
this as anything else than me being careful with what goes into the main
sources. Feel free to tell me I'm wrong, if you think I am.

o Instead of setting the internal 'set.use_netrc' variable to -1, 1 or 0, I
  would prefer to have it use defines/enums with descriptive names.
  It makes it less confusing in the code where it is later used.

o The removal of the line (url.c around 1320):
  conn->bits.user_passwd = data->set.userpwd?1:0;

  is that *really* thought through and motivated? Can you please elaborate on
  why this isn't needed for cases where CURLOPT_NETRC is not used.

o The comment that says "There could be a colon in the user name / password
  part" is wrong. Colons are "reserved" in that part of the URL and may
  only be used as delimiters. If user names or passwords contain colons, they
  must be URL encoded. (RFC2396 section 3.2.2)

  Thus, using strrchr() instead of strchr() to find the colon isn't needed
  but of course does no harm.

o The src/main.c patch that introduces a different meaning of -n when used
  twice is not that nice imho. The curl tool has no other option that gets
  such a different meaning when used twice. I really can't see the need for
  this either, this new netrc stuff is mostly for library uses when you want
  to override what the URL contains, while a user of a command line tool will
  most often control both the options and the URL.

  If the new functionality really is wanted from the command line, I suggest
  we use a new (long) option instead and keep the old functionality as it is
  today.

> - WARNING WARNING DANGER WILL ROBINSON
> - the test cases 130 .. 133 (the new ones) will destroy your .netrc
> - given that they test .netrc reading, I don't know how to avoid that

*Ugha*. While I of course appreciate the provided test cases, we really
cannot have this behavior there.

A suggested approach to fix this: we introduce a "hack" in libcurl that when
compiled with --enable-debug changes the behavior of Curl_parsenetrc()
slightly, and if a certain environment variable is set, it will identify
which file to use instead of the one that would normally be used.

Then, in a similar style which we check for memory debugging in the test
suite, we check for netrc debugging as well and we only run the netrc test
cases if curl is compiled to work with them.

I'm open for other ideas. We MUST NOT modify a single file outside of the
tests directory.

> Test-case 1 (HTTP) fails with an error I don't understand, which is
> attached.

I didn't find any error text attached. Did you forget to include that or did
I miss it?

-- 
    Daniel Stenberg -- curl groks URLs -- http://curl.haxx.se/