[PATCH] submodule--helper: normalize funny urls

[PATCH] submodule--helper: normalize funny urls

[Stefan Beller]

Currently a URL for the superproject ending in

(A)    .../path/to/dir
(B)    .../path/to/dir/
(C)    .../path/to/dir/.
(D)    .../path/to/dir/./.
(E)    .../path/to/dir/.///.//.

is treated the same in (A) and (B), but (C, D, E) are different.

We never produce the URLs in (C,D,E) ourselves, they come to use, because
the user used it as the URL for cloning a superproject.
Normalize these paths.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

 By being strict in Git, I think we also fix the Git for Windows painpoints.
 
 This goes on top of origin/sb/submodule-ignore-trailing-slash.
 
 Thanks,
 Stefan

 builtin/submodule--helper.c | 49 ++++++++++++++++++++++++++++++++++-----------
 t/t0060-path-utils.sh       | 11 ++++++----
 2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 260f46f..ca90763 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -76,6 +76,30 @@ static int chop_last_dir(char **remoteurl, int is_relative)
 	return 0;
 }
 
+static void strip_url_ending(char *url, size_t *_len)
+{
+	int check_url_stripping = 1;
+	size_t len = _len ? *_len : strlen(url);
+
+	while (check_url_stripping) {
+		check_url_stripping = 0;
+		if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
+			url[len-2] = '\0';
+			len -= 2;
+			check_url_stripping = 1;
+		}
+
+		if (is_dir_sep(url[len-1])) {
+			url[len-1] = '\0';
+			len --;
+			check_url_stripping = 1;
+		}
+	}
+
+	if (_len)
+		*_len = len;
+}
+
 /*
  * The `url` argument is the URL that navigates to the submodule origin
  * repo. When relative, this URL is relative to the superproject origin
@@ -93,14 +117,16 @@ static int chop_last_dir(char **remoteurl, int is_relative)
  * the superproject working tree otherwise.
  *
  * NEEDSWORK: This works incorrectly on the domain and protocol part.
- * remote_url      url              outcome          expectation
- * http://a.com/b  ../c             http://a.com/c   as is
- * http://a.com/b/ ../c             http://a.com/c   same as previous line, but
- *                                                   ignore trailing slash in url
- * http://a.com/b  ../../c          http://c         error out
- * http://a.com/b  ../../../c       http:/c          error out
- * http://a.com/b  ../../../../c    http:c           error out
- * http://a.com/b  ../../../../../c    .:c           error out
+ * remote_url       url              outcome          expectation
+ * http://a.com/b   ../c             http://a.com/c   as is
+ * http://a.com/b/  ../c             http://a.com/c   same as previous line, but
+ *                                                    ignore trailing '/' in url
+ * http://a.com/b/. ../c             http://a.com/c   same as previous line, but
+ *                                                    ignore trailing '/.' in url
+ * http://a.com/b   ../../c          http://c         error out
+ * http://a.com/b   ../../../c       http:/c          error out
+ * http://a.com/b   ../../../../c    http:c           error out
+ * http://a.com/b   ../../../../../c    .:c           error out
  * NEEDSWORK: Given how chop_last_dir() works, this function is broken
  * when a local part has a colon in its path component, too.
  */
@@ -115,8 +141,7 @@ static char *relative_url(const char *remote_url,
 	struct strbuf sb = STRBUF_INIT;
 	size_t len = strlen(remoteurl);
 
-	if (is_dir_sep(remoteurl[len-1]))
-		remoteurl[len-1] = '\0';
+	strip_url_ending(remoteurl, &len);
 
 	if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
 		is_relative = 0;
@@ -149,10 +174,10 @@ static char *relative_url(const char *remote_url,
 	}
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
-	if (ends_with(url, "/"))
-		strbuf_setlen(&sb, sb.len - 1);
 	free(remoteurl);
 
+	strip_url_ending(sb.buf, &sb.len);
+
 	if (starts_with_dot_slash(sb.buf))
 		out = xstrdup(sb.buf + 2);
 	else
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 25b48e5..e154e5f 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -329,14 +329,17 @@ test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
 test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
 test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
 test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
-test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
+test_submodule_relative_url "(null)" "$PWD/sub/." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/.////././/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)"
 test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
 test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö"
-test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub" "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub/." "../submodule" "$(pwd)/submodule"
 test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
 test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
-test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
+test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo"
 test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
 test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
 test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
-- 
2.10.1.480.g573bd76

Re: [PATCH] submodule--helper: normalize funny urls

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> +static void strip_url_ending(char *url, size_t *_len)
> +{
> +	int check_url_stripping = 1;
> +	size_t len = _len ? *_len : strlen(url);
> +
> +	while (check_url_stripping) {
> +		check_url_stripping = 0;
> +		if (is_dir_sep(url[len-2]) && url[len-1] == '.') {

This is "strip /. at the end" it seems.

Does anything in the loop control guarantees 2 <= len at this point?

> +			url[len-2] = '\0';
> +			len -= 2;
> +			check_url_stripping = 1;
> +		}
> +
> +		if (is_dir_sep(url[len-1])) {

This is "strip / at the end" it seems.

Does anything in the loop control guarantees 1 <= len at this point?

> +			url[len-1] = '\0';
> +			len--;
> +			check_url_stripping = 1;
> +		}
> +	}

Re: [PATCH] submodule--helper: normalize funny urls

[Stefan Beller]

On Mon, Oct 17, 2016 at 3:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> +static void strip_url_ending(char *url, size_t *_len)
>> +{
>> +     int check_url_stripping = 1;
>> +     size_t len = _len ? *_len : strlen(url);
>> +
>> +     while (check_url_stripping) {
>> +             check_url_stripping = 0;
>> +             if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
>
> This is "strip /. at the end" it seems.
>
> Does anything in the loop control guarantees 2 <= len at this point?

Oh, thanks for pointing that out. I thought about that and missed to add it.
I'll reroll with the length check once we hear back from Windows folks,
that this is a viable strategy for them, too.

Thanks,
Stefan

>
>> +                     url[len-2] = '\0';
>> +                     len -= 2;
>> +                     check_url_stripping = 1;
>> +             }
>> +
>> +             if (is_dir_sep(url[len-1])) {
>
> This is "strip / at the end" it seems.
>
> Does anything in the loop control guarantees 1 <= len at this point?
>
>> +                     url[len-1] = '\0';
>> +                     len--;
>> +                     check_url_stripping = 1;
>> +             }
>> +     }

Re: [PATCH] submodule--helper: normalize funny urls

[Johannes Schindelin]

Hi Stefan,

On Mon, 17 Oct 2016, Stefan Beller wrote:

> On Mon, Oct 17, 2016 at 3:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> +static void strip_url_ending(char *url, size_t *_len)
> >> +{
> >> +     int check_url_stripping = 1;
> >> +     size_t len = _len ? *_len : strlen(url);
> >> +
> >> +     while (check_url_stripping) {
> >> +             check_url_stripping = 0;
> >> +             if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
> >
> > This is "strip /. at the end" it seems.
> >
> > Does anything in the loop control guarantees 2 <= len at this point?
> 
> Oh, thanks for pointing that out. I thought about that and missed to add it.
> I'll reroll with the length check once we hear back from Windows folks,
> that this is a viable strategy for them, too.

It is a viable strategy for me, too.

Thanks,
Dscho

Re: [PATCH] submodule--helper: normalize funny urls

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Currently a URL for the superproject ending in
>
> (A)    .../path/to/dir
> (B)    .../path/to/dir/
> (C)    .../path/to/dir/.
> (D)    .../path/to/dir/./.
> (E)    .../path/to/dir/.///.//.
>
> is treated the same in (A) and (B), but (C, D, E) are different.

You may know what you meant to say with "treated", but the readers
do not know "treated" in what situation you are talking about.  We
need to tell the readers that the bug being fixed is about resolving
a relative URL "../<something>" off of the URL of the superproject
to compute the remote URL for a submodule repository.

> We never produce the URLs in (C,D,E) ourselves, they come to use, because
> the user used it as the URL for cloning a superproject.
> Normalize these paths.

As you know the externally-visible impact of this change (which I
asked you, but didn't see an on-list answer to, by the way), please
describe what this means to the end user in the log message.  It is
normally done in an earlier part of the log message to describe the
problem being solved and its background.

If I understand the issue correctly, it may go like this:

	The remote URL for the submodule can be specified relative
	to the URL of the superproject in .gitmodules.  A top-level
	git://site.xz/toplevel.git can specify in its .gitmodules

		[submodule "sub"]
			url = ../submodule.git
			path = sub

	to say that git://site.xz/submodule.git is where the
	submodule bound at its "sub/" is found.

	However, when the toplevel is cloned like this:

		git clone git://site.xz/toplevel.git/. top

	i.e. the toplevel specifies its URL with trailing "/.", the
	code set the URL to git://site.xz/toplevel.git/submodule.git
	for the submodule, which is nonsense.  This was because the
	logic failed to treat trailing "/." any differently from
	trailing "/<anything-without-slash>" when resolving a
	relative URL "../<something>" off of it.  Stripping "/." at
	the end does *not* take you one level up, even though
	stripping "/<anything-without-slash>" does!
	
And then describe the solution/bugfix, listing A-C (or A-E) and
telling that these will be treated the same way.

Re: [PATCH] submodule--helper: normalize funny urls

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Stefan Beller <sbeller@google.com> writes:
>
>> +static void strip_url_ending(char *url, size_t *_len)
>> +{
>> +	int check_url_stripping = 1;
>> +	size_t len = _len ? *_len : strlen(url);
>> +
>> +	while (check_url_stripping) {
>> +		check_url_stripping = 0;
>> +		if (is_dir_sep(url[len-2]) && url[len-1] == '.') {
>
> This is "strip /. at the end" it seems.
>
> Does anything in the loop control guarantees 2 <= len at this point?
>
>> +			url[len-2] = '\0';
>> +			len -= 2;
>> +			check_url_stripping = 1;
>> +		}
>> +
>> +		if (is_dir_sep(url[len-1])) {
>
> This is "strip / at the end" it seems.
>
> Does anything in the loop control guarantees 1 <= len at this point?
>
>> +			url[len-1] = '\0';
>> +			len--;
>> +			check_url_stripping = 1;
>> +		}
>> +	}

I also somehow find the "check-url-stripping" variable ugly.

	while (URL still has something that could be stripped) {
		if (ends with "/.") {
			strip "/.";
			continue;
		}
		if (ends with "/") {
			strip "/";
			continue;
		}
		break;
	}

perhaps?

Re: [PATCH] submodule--helper: normalize funny urls

[Stefan Beller]

On Tue, Oct 18, 2016 at 10:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I also somehow find the "check-url-stripping" variable ugly.
>
>         while (URL still has something that could be stripped) {

for(;;) {

here ? (this code would not need a variable, and
for wins over while:
$ git grep "while (1)" |wc -l
107
$ git grep "for (;;)" |wc -l
128
)

>                 if (ends with "/.") {
>                         strip "/.";
>                         continue;
>                 }
>                 if (ends with "/") {
>                         strip "/";
>                         continue;
>                 }
>                 break;
>         }
>
> perhaps?

Re: [PATCH] submodule--helper: normalize funny urls

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> for(;;) {
>
> here ? (this code would not need a variable, and
> for wins over while:
> $ git grep "while (1)" |wc -l
> 107
> $ git grep "for (;;)" |wc -l
> 128
> )

I dunno; the numbers tells me there is no strong preference by wide
margin either way.

I am not sure if the end shape does not really need an exit
condition.  If there is a need for one, "for (; condition;)" would
look strange.  If there isn't, "for (;;)" is actually my personal
preference over "while (1)".

[PATCH] submodule--helper: normalize funny urls

[Stefan Beller]

The remote URL for the submodule can be specified relative
to the URL of the superproject in .gitmodules.  A top-level
git://site.xz/toplevel.git can specify in its .gitmodules

        [submodule "sub"]
                url = ../submodule.git
                path = sub

to say that git://site.xz/submodule.git is where the
submodule bound at its "sub/" is found.

However, when the toplevel is cloned like this:

        git clone git://site.xz/toplevel.git/. top

i.e. the toplevel specifies its URL with trailing "/.", the
code set the URL to git://site.xz/toplevel.git/submodule.git
for the submodule, which is nonsense.  This was because the
logic failed to treat trailing "/." any differently from
trailing "/<anything-without-slash>" when resolving a
relative URL "../<something>" off of it.  Stripping "/." at
the end does *not* take you one level up, even though
stripping "/<anything-without-slash>" does!

Some users may rely on this by always cloning with '/.' and having
an additional '../' in the relative path for the submodule, and this
patch breaks them. So why introduce this patch?

The fix in c12922024 (submodule: ignore trailing slash on superproject
URL, 2016-10-10) and prior discussion revealed, that Git and Git
for Windows treat URLs differently, as currently Git for Windows
strips off a trailing dot from paths when calling a Git binary
unlike when running a shell. Which means Git for Windows is already
doing the right thing for the case mentioned above, but it would fail
our current tests, that test for the broken behavior and it would
confuse users working across platforms. So we'd rather fix it
in Git to ignore any of these trailing no ops in the path properly.

We never produce the URLs with a trailing '/.' in Git ourselves,
they come to us, because the user used it as the URL for cloning
a superproject. Normalize these paths.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

 * reworded the commit message, taken from Junio, but added more explanation
   why we want to introduce this patch. 
 * added the length check
 * use an infinite loop with break instead of a variable
   to determine the ending condition.

 builtin/submodule--helper.c | 48 +++++++++++++++++++++++++++++++++------------
 t/t0060-path-utils.sh       | 11 +++++++----
 2 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 260f46f..ac03cb3 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -76,6 +76,29 @@ static int chop_last_dir(char **remoteurl, int is_relative)
 	return 0;
 }
 
+static void strip_url_ending(char *url, size_t *_len)
+{
+	size_t len = _len ? *_len : strlen(url);
+
+	for (;;) {
+		if (len > 1 && is_dir_sep(url[len-2]) && url[len-1] == '.') {
+			url[len-2] = '\0';
+			len -= 2;
+			continue;
+		}
+		if (len > 0 && is_dir_sep(url[len-1])) {
+			url[len-1] = '\0';
+			len --;
+			continue;
+		}
+
+		break;
+	}
+
+	if (_len)
+		*_len = len;
+}
+
 /*
  * The `url` argument is the URL that navigates to the submodule origin
  * repo. When relative, this URL is relative to the superproject origin
@@ -93,14 +116,16 @@ static int chop_last_dir(char **remoteurl, int is_relative)
  * the superproject working tree otherwise.
  *
  * NEEDSWORK: This works incorrectly on the domain and protocol part.
- * remote_url      url              outcome          expectation
- * http://a.com/b  ../c             http://a.com/c   as is
- * http://a.com/b/ ../c             http://a.com/c   same as previous line, but
- *                                                   ignore trailing slash in url
- * http://a.com/b  ../../c          http://c         error out
- * http://a.com/b  ../../../c       http:/c          error out
- * http://a.com/b  ../../../../c    http:c           error out
- * http://a.com/b  ../../../../../c    .:c           error out
+ * remote_url       url              outcome          expectation
+ * http://a.com/b   ../c             http://a.com/c   as is
+ * http://a.com/b/  ../c             http://a.com/c   same as previous line, but
+ *                                                    ignore trailing '/' in url
+ * http://a.com/b/. ../c             http://a.com/c   same as previous line, but
+ *                                                    ignore trailing '/.' in url
+ * http://a.com/b   ../../c          http://c         error out
+ * http://a.com/b   ../../../c       http:/c          error out
+ * http://a.com/b   ../../../../c    http:c           error out
+ * http://a.com/b   ../../../../../c    .:c           error out
  * NEEDSWORK: Given how chop_last_dir() works, this function is broken
  * when a local part has a colon in its path component, too.
  */
@@ -115,8 +140,7 @@ static char *relative_url(const char *remote_url,
 	struct strbuf sb = STRBUF_INIT;
 	size_t len = strlen(remoteurl);
 
-	if (is_dir_sep(remoteurl[len-1]))
-		remoteurl[len-1] = '\0';
+	strip_url_ending(remoteurl, &len);
 
 	if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))
 		is_relative = 0;
@@ -149,10 +173,10 @@ static char *relative_url(const char *remote_url,
 	}
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url);
-	if (ends_with(url, "/"))
-		strbuf_setlen(&sb, sb.len - 1);
 	free(remoteurl);
 
+	strip_url_ending(sb.buf, &sb.len);
+
 	if (starts_with_dot_slash(sb.buf))
 		out = xstrdup(sb.buf + 2);
 	else
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 25b48e5..e154e5f 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -329,14 +329,17 @@ test_submodule_relative_url "(null)" "./foo" "../submodule" "submodule"
 test_submodule_relative_url "(null)" "//somewhere else/repo" "../subrepo" "//somewhere else/subrepo"
 test_submodule_relative_url "(null)" "$PWD/subsuper_update_r" "../subsubsuper_update_r" "$(pwd)/subsubsuper_update_r"
 test_submodule_relative_url "(null)" "$PWD/super_update_r2" "../subsuper_update_r" "$(pwd)/subsuper_update_r"
-test_submodule_relative_url "(null)" "$PWD/." "../." "$(pwd)/."
-test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)/."
+test_submodule_relative_url "(null)" "$PWD/sub/." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD/sub/.////././/./." "../." "$(pwd)"
+test_submodule_relative_url "(null)" "$PWD" "./." "$(pwd)"
 test_submodule_relative_url "(null)" "$PWD/addtest" "../repo" "$(pwd)/repo"
 test_submodule_relative_url "(null)" "$PWD" "./å äö" "$(pwd)/å äö"
-test_submodule_relative_url "(null)" "$PWD/." "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub" "../submodule" "$(pwd)/submodule"
+test_submodule_relative_url "(null)" "$PWD/sub/." "../submodule" "$(pwd)/submodule"
 test_submodule_relative_url "(null)" "$PWD/submodule" "../submodule" "$(pwd)/submodule"
 test_submodule_relative_url "(null)" "$PWD/home2/../remote" "../bundle1" "$(pwd)/home2/../bundle1"
-test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo/."
+test_submodule_relative_url "(null)" "$PWD/submodule_update_repo" "./." "$(pwd)/submodule_update_repo"
 test_submodule_relative_url "(null)" "file:///tmp/repo" "../subrepo" "file:///tmp/subrepo"
 test_submodule_relative_url "(null)" "foo/bar" "../submodule" "foo/submodule"
 test_submodule_relative_url "(null)" "foo" "../submodule" "submodule"
-- 
2.10.1.480.g573bd76

Re: [PATCH] submodule--helper: normalize funny urls

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Some users may rely on this by always cloning with '/.' and having
> an additional '../' in the relative path for the submodule, and this
> patch breaks them. So why introduce this patch?
>
> The fix in c12922024 (submodule: ignore trailing slash on superproject
> URL, 2016-10-10) and prior discussion revealed, that Git and Git
> for Windows treat URLs differently, as currently Git for Windows
> strips off a trailing dot from paths when calling a Git binary
> unlike when running a shell. Which means Git for Windows is already
> doing the right thing for the case mentioned above, but it would fail
> our current tests, that test for the broken behavior and it would
> confuse users working across platforms. So we'd rather fix it
> in Git to ignore any of these trailing no ops in the path properly.
>
> We never produce the URLs with a trailing '/.' in Git ourselves,
> they come to us, because the user used it as the URL for cloning
> a superproject. Normalize these paths.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
>  * reworded the commit message, taken from Junio, but added more explanation
>    why we want to introduce this patch. 

The additional explanation is very good.

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 260f46f..ac03cb3 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -76,6 +76,29 @@ static int chop_last_dir(char **remoteurl, int is_relative)
>  	return 0;
>  }
>  
> +static void strip_url_ending(char *url, size_t *_len)
> +{
> +	size_t len = _len ? *_len : strlen(url);

Stare at our codebase and you'd notice that we avoid using names
with leading underscore deliberately and use trailing one instead
when we name a throw-away name like this.  Let's do the same here.
I.e.

	static void strip_url_ending(char *url, size_t *len_)
	{
		size_t len = len_ ? *len_ : strlen(url);

> +	for (;;) {
> +		if (len > 1 && is_dir_sep(url[len-2]) && url[len-1] == '.') {
> +			url[len-2] = '\0';

"len-1" and "len-2" are usually spelled with SP on both sides of
binary operators.

> +			len -= 2;
> +			continue;
> +		}
> +		if (len > 0 && is_dir_sep(url[len-1])) {
> +			url[len-1] = '\0';
> +			len --;

And post-decrement sticks to whatever it is decrementing without SP.

> +			continue;
> +		}