credential: fix matching URLs with multiple levels in path

46fd7b3900 ("credential: allow wildcard patterns when matching config",
2020-02-20) introduced support for matching credential helpers using
urlmatch.  In doing so, it introduced code to percent-encode the paths
we get from the credential helper so that they could be effectively
matched by the urlmatch code.

Unfortunately, that code had a bug: it percent-encoded the slashes in
the path, resulting in any URL path that contained multiple levels
(i.e., a directory component) not matching.

We are currently the only caller of the percent-encoding code and could
simply change it not to encode slashes.  However, we still want to
encode slashes in the username component, so we need to have both
behaviors available.

So instead, let's add a flag to control encoding slashes, which is the
behavior we want here, and use it when calling the code in this case.

Add a test for credential helper URLs using multiple slashes in the
path, which our test suite previously lacked, as well as one ensuring
that we handle usernames with slashes gracefully.  Since we're testing
other percent-encoding handling, let's add one for non-ASCII UTF-8
characters as well.

Reported-by: Ilya Tretyakov <it@it3xl.ru>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/credential.c b/credential.c
index 77dfde44e3d136a29f68091878c9f9392af746c0..8348b6fb5718e241842f265e1f5464cdd1aaf7d1 100644 (file)
--- a/credential.c
+++ b/credential.c
@@ -131,14 +131,14 @@ static void credential_format(struct credential *c, struct strbuf *out)
                return;
        strbuf_addf(out, "%s://", c->protocol);
        if (c->username && *c->username) {
-               strbuf_add_percentencode(out, c->username);
+               strbuf_add_percentencode(out, c->username, STRBUF_ENCODE_SLASH);
                strbuf_addch(out, '@');
        }
        if (c->host)
                strbuf_addstr(out, c->host);
        if (c->path) {
                strbuf_addch(out, '/');
-               strbuf_add_percentencode(out, c->path);
+               strbuf_add_percentencode(out, c->path, 0);
        }
 }
 

diff --git a/strbuf.c b/strbuf.c
index bb0065ccaf5b764323cd727dbabe3061bbe328e4..51c83aae2d24bbbad5384b653d0c60b52ec4f4a6 100644 (file)
--- a/strbuf.c
+++ b/strbuf.c
@@ -479,15 +479,17 @@ void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src)
        }
 }
 
-#define URL_UNSAFE_CHARS " <>\"%{}|\\^`:/?#[]@!$&'()*+,;="
+#define URL_UNSAFE_CHARS " <>\"%{}|\\^`:?#[]@!$&'()*+,;="
 
-void strbuf_add_percentencode(struct strbuf *dst, const char *src)
+void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags)
 {
        size_t i, len = strlen(src);
 
        for (i = 0; i < len; i++) {
                unsigned char ch = src[i];
-               if (ch <= 0x1F || ch >= 0x7F || strchr(URL_UNSAFE_CHARS, ch))
+               if (ch <= 0x1F || ch >= 0x7F ||
+                   (ch == '/' && (flags & STRBUF_ENCODE_SLASH)) ||
+                   strchr(URL_UNSAFE_CHARS, ch))
                        strbuf_addf(dst, "%%%02X", (unsigned char)ch);
                else
                        strbuf_addch(dst, ch);

diff --git a/strbuf.h b/strbuf.h
index 2504ae4ecb6e4575f2ef3e80338556caefaaaa4c..5d20942c045f8bea436b30bd48c754b404ca2e78 100644 (file)
--- a/strbuf.h
+++ b/strbuf.h
@@ -366,11 +366,16 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb,
  */
 void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src);
 
+#define STRBUF_ENCODE_SLASH 1
+
 /**
  * Append the contents of a string to a strbuf, percent-encoding any characters
  * that are needed to be encoded for a URL.
+ *
+ * If STRBUF_ENCODE_SLASH is set in flags, percent-encode slashes.  Otherwise,
+ * slashes are not percent-encoded.
  */
-void strbuf_add_percentencode(struct strbuf *dst, const char *src);
+void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags);
 
 /**
  * Append the given byte size as a human-readable string (i.e. 12.23 KiB,

diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 39f097ea9e5094530b981c49193df8df2cbda406..08799b41251c9889113df197ca6b79fae6481564 100755 (executable)
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -291,6 +291,51 @@ test_expect_success 'match percent-encoded values' '
        EOF
 '
 
+test_expect_success 'match percent-encoded UTF-8 values in path' '
+       test_config credential.https://example.com.useHttpPath true &&
+       test_config credential.https://example.com/perú.git.helper "$HELPER" &&
+       check fill <<-\EOF
+       url=https://example.com/per%C3%BA.git
+       --
+       protocol=https
+       host=example.com
+       path=perú.git
+       username=foo
+       password=bar
+       --
+       EOF
+'
+
+test_expect_success 'match percent-encoded values in username' '
+       test_config credential.https://user%2fname@example.com/foo/bar.git.helper "$HELPER" &&
+       check fill <<-\EOF
+       url=https://user%2fname@example.com/foo/bar.git
+       --
+       protocol=https
+       host=example.com
+       username=foo
+       password=bar
+       --
+       EOF
+'
+
+test_expect_success 'fetch with multiple path components' '
+       test_unconfig credential.helper &&
+       test_config credential.https://example.com/foo/repo.git.helper "verbatim foo bar" &&
+       check fill <<-\EOF
+       url=https://example.com/foo/repo.git
+       --
+       protocol=https
+       host=example.com
+       username=foo
+       password=bar
+       --
+       verbatim: get
+       verbatim: protocol=https
+       verbatim: host=example.com
+       EOF
+'
+
 test_expect_success 'pull username from config' '
        test_config credential.https://example.com.username foo &&
        check fill <<-\EOF