Re: [RFC PATCH] credential: minor documentation fixes

["Carlo Marcelo Arenas Belón" <carenas@gmail.com>]

On Mon, May 04, 2020 at 10:44:36AM -0400, Jeff King wrote:
> On Mon, May 04, 2020 at 12:45:20AM -0700, Carlo Marcelo Arenas Belón wrote:
> > 
> > * the meaning of "exactly" for matching protocol and hostname in the URL
> >   since 06 are both case insensitive per RFC3986 and we have been
> >   ambiguous on that, leading to some helpers assuming case or encoding.
> 
> Yeah, IIRC we discussed case-sensitivity at the time and went with the
> stricter behavior in the name of safety over convenience. And I don't
> think anybody has complained since then. So I'm not really _opposed_ to
> loosening it to match the URL, but perhaps a maintenance release is not
> the best time to do so.

agree, but I was talking not in the context of a feature, but on how we
are to define the interaction with helpers (which was meant to be part of
this maintenance release).

currently (since it is undefined) a naive helper could do a caseless match
by assuming we really meant url as defined by RFC3986, and therefore affect
the wrong credential by the operation.

indeed; our own code might get confused so maybe something like (not fully
tested and likely to need some test coverage) will make sense then as part
of this maintenance release, with some additional mention that clarifies
we REALLY meant "exactly" so that helpers can be updated?

Carlo
---
diff --git a/credential.c b/credential.c
index 108d9e183a..d2c879a9b3 100644
--- a/credential.c
+++ b/credential.c
@@ -70,7 +70,7 @@ static int proto_is_http(const char *s)
 {
 	if (!s)
 		return 0;
-	return !strcmp(s, "https") || !strcmp(s, "http");
+	return !strcasecmp(s, "https") || !strcasecmp(s, "http");
 }
 
 static void credential_describe(struct credential *c, struct strbuf *out);
diff --git a/fsck.c b/fsck.c
index 73f30773f2..d779acdae8 100644
--- a/fsck.c
+++ b/fsck.c
@@ -997,10 +997,10 @@ static int url_to_curl_url(const char *url, const char **out)
 	    skip_prefix(url, "ftp::", out) ||
 	    skip_prefix(url, "ftps::", out))
 		return 1;
-	if (starts_with(url, "http://") ||
-	    starts_with(url, "https://") ||
-	    starts_with(url, "ftp://") ||
-	    starts_with(url, "ftps://")) {
+	if (istarts_with(url, "http://") ||
+	    istarts_with(url, "https://") ||
+	    istarts_with(url, "ftp://") ||
+	    istarts_with(url, "ftps://")) {
 		*out = url;
 		return 1;
 	}