[PATCH] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[PATCH] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

using a base tag has the side-effect of not just changing the few URLs
of gitweb's static resources, but all other relative links (e.g. those
in a README.html), too.

Signed-off-by: Tobias Girstmair <tobi@isticktoit.net>
---
My exact use case is this: Many of my repositories have README.{rst,md}
files with relative links to more md/rst files. I'm generating a
README.html from that file with a post-update hook, and rewrite links,
so they display the file. Some cloned repos' READMEs even have images in
them, whose links I'm fixing up as well.

Due to the precence of the <base> tag, I can't just prepend
"blob_plain/HEAD:" to the URLs, but have to recreate the full URL,
including the last few directories of $GIT_DIR. That's annoying to keep
portable (move a repo from e.g. /srv/git/foo.git to
/srv/git/**public**/foo.git and the URL must be changed in the hook, and
the hook re-executed).

Looking forward to your feedback,
	tobias

 gitweb/gitweb.perl | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0959a78..049bb64 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2232,7 +2232,7 @@ sub git_get_avatar {
 		return $pre_white .
 		       "<img width=\"$size\" " .
 		            "class=\"avatar\" " .
-		            "src=\"".esc_url($url)."\" " .
+		            "src=\"".get_base_url().esc_url($url)."\" " .
 			    "alt=\"\" " .
 		       "/>" . $post_white;
 	} else {
@@ -2674,6 +2674,13 @@ sub get_feed_info {
 	return %res;
 }
 
+# the stylesheet, favicon etc urls won't work correctly with path_info
+# unless we set the appropriate base URL. not using a <base> tag to not
+# also change relative URLs inserted by the user.
+sub get_base_url {
+	return $ENV{'PATH_INFO'}? esc_url($base_url)."/" : "";
+}
+
 ## ----------------------------------------------------------------------
 ## git utility subroutines, invoking git commands
 
@@ -4099,17 +4106,17 @@ sub print_header_links {
 	# print out each stylesheet that exist, providing backwards capability
 	# for those people who defined $stylesheet in a config file
 	if (defined $stylesheet) {
-		print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
+		print '<link rel="stylesheet" type="text/css" href="'.get_base_url().esc_url($stylesheet).'"/>'."\n";
 	} else {
 		foreach my $stylesheet (@stylesheets) {
 			next unless $stylesheet;
-			print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
+			print '<link rel="stylesheet" type="text/css" href="'.get_base_url().esc_url($stylesheet).'"/>'."\n";
 		}
 	}
 	print_feed_meta()
 		if ($status eq '200 OK');
 	if (defined $favicon) {
-		print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
+		print qq(<link rel="shortcut icon" href=").get_base_url().esc_url($favicon).qq(" type="image/png" />\n);
 	}
 }
 
@@ -4212,11 +4219,6 @@ sub git_header_html {
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
 EOF
-	# the stylesheet, favicon etc urls won't work correctly with path_info
-	# unless we set the appropriate base URL
-	if ($ENV{'PATH_INFO'}) {
-		print "<base href=\"".esc_url($base_url)."\" />\n";
-	}
 	print_header_links($status);
 
 	if (defined $site_html_head_string) {
@@ -4234,7 +4236,7 @@ sub git_header_html {
 	if (defined $logo) {
 		print $cgi->a({-href => esc_url($logo_url),
 		               -title => $logo_label},
-		              $cgi->img({-src => esc_url($logo),
+		              $cgi->img({-src => get_base_url().esc_url($logo),
 		                         -width => 72, -height => 27,
 		                         -alt => "git",
 		                         -class => "logo"}));
@@ -4299,7 +4301,7 @@ sub git_footer_html {
 		insert_file($site_footer);
 	}
 
-	print qq!<script type="text/javascript" src="!.esc_url($javascript).qq!"></script>\n!;
+	print qq!<script type="text/javascript" src="!.get_base_url().esc_url($javascript).qq!"></script>\n!;
 	if (defined $action &&
 	    $action eq 'blame_incremental') {
 		print qq!<script type="text/javascript">\n!.
@@ -8273,7 +8275,7 @@ sub git_feed {
 		if (defined $logo || defined $favicon) {
 			# prefer the logo to the favicon, since RSS
 			# doesn't allow both
-			my $img = esc_url($logo || $favicon);
+			my $img = get_base_url().esc_url($logo || $favicon);
 			print "<image>\n" .
 			      "<url>$img</url>\n" .
 			      "<title>$title</title>\n" .
@@ -8299,11 +8301,11 @@ sub git_feed {
 		      # use project owner for feed author
 		      "<author><name>$owner</name></author>\n";
 		if (defined $favicon) {
-			print "<icon>" . esc_url($favicon) . "</icon>\n";
+			print "<icon>" . get_base_url() . esc_url($favicon) . "</icon>\n";
 		}
 		if (defined $logo) {
 			# not twice as wide as tall: 72 x 27 pixels
-			print "<logo>" . esc_url($logo) . "</logo>\n";
+			print "<logo>" . get_base_url() . esc_url($logo) . "</logo>\n";
 		}
 		if (! %latest_date) {
 			# dummy date to keep the feed valid until commits trickle in:
-- 
2.21.3

[PATCH v2] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

using a base tag has the side-effect of not just changing the few URLs
of gitweb's static resources, but all other relative links (e.g. those
in a README.html), too.

Signed-off-by: Tobias Girstmair <tobi@isticktoit.net>
---
In v1 of this patch I didn't think about already absolute URLs. Fixed.

	tobias

 gitweb/gitweb.perl | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0959a78..6a4f2fa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1616,6 +1616,19 @@ sub esc_url {
 	return $str;
 }
 
+# the stylesheet, favicon etc urls won't work correctly with path_info
+# unless we set the appropriate base URL. not using a <base> tag to not
+# also change relative URLs inserted by the user.
+sub esc_url_base {
+	my $url = shift;
+	my $prefix = $ENV{'PATH_INFO'}? esc_url($base_url)."/" : "";
+	if (url !~ m{^(https?:)?//?}) {
+		return $prefix . esc_url($url);
+	} else {
+		return esc_url($url);
+	}
+}
+
 # quote unsafe characters in HTML attributes
 sub esc_attr {
 
@@ -2232,7 +2245,7 @@ sub git_get_avatar {
 		return $pre_white .
 		       "<img width=\"$size\" " .
 		            "class=\"avatar\" " .
-		            "src=\"".esc_url($url)."\" " .
+		            "src=\"".esc_url_base($url)."\" " .
 			    "alt=\"\" " .
 		       "/>" . $post_white;
 	} else {
@@ -4099,17 +4112,17 @@ sub print_header_links {
 	# print out each stylesheet that exist, providing backwards capability
 	# for those people who defined $stylesheet in a config file
 	if (defined $stylesheet) {
-		print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
+		print '<link rel="stylesheet" type="text/css" href="'.esc_url_base($stylesheet).'"/>'."\n";
 	} else {
 		foreach my $stylesheet (@stylesheets) {
 			next unless $stylesheet;
-			print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
+			print '<link rel="stylesheet" type="text/css" href="'.esc_url_base($stylesheet).'"/>'."\n";
 		}
 	}
 	print_feed_meta()
 		if ($status eq '200 OK');
 	if (defined $favicon) {
-		print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
+		print qq(<link rel="shortcut icon" href=").esc_url_base($favicon).qq(" type="image/png" />\n);
 	}
 }
 
@@ -4212,11 +4225,6 @@ sub git_header_html {
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
 EOF
-	# the stylesheet, favicon etc urls won't work correctly with path_info
-	# unless we set the appropriate base URL
-	if ($ENV{'PATH_INFO'}) {
-		print "<base href=\"".esc_url($base_url)."\" />\n";
-	}
 	print_header_links($status);
 
 	if (defined $site_html_head_string) {
@@ -4234,7 +4242,7 @@ sub git_header_html {
 	if (defined $logo) {
 		print $cgi->a({-href => esc_url($logo_url),
 		               -title => $logo_label},
-		              $cgi->img({-src => esc_url($logo),
+		              $cgi->img({-src => esc_url_base($logo),
 		                         -width => 72, -height => 27,
 		                         -alt => "git",
 		                         -class => "logo"}));
@@ -4299,7 +4307,7 @@ sub git_footer_html {
 		insert_file($site_footer);
 	}
 
-	print qq!<script type="text/javascript" src="!.esc_url($javascript).qq!"></script>\n!;
+	print qq!<script type="text/javascript" src="!.esc_url_base($javascript).qq!"></script>\n!;
 	if (defined $action &&
 	    $action eq 'blame_incremental') {
 		print qq!<script type="text/javascript">\n!.
@@ -8273,7 +8281,7 @@ sub git_feed {
 		if (defined $logo || defined $favicon) {
 			# prefer the logo to the favicon, since RSS
 			# doesn't allow both
-			my $img = esc_url($logo || $favicon);
+			my $img = esc_url_base($logo || $favicon);
 			print "<image>\n" .
 			      "<url>$img</url>\n" .
 			      "<title>$title</title>\n" .
@@ -8299,11 +8307,11 @@ sub git_feed {
 		      # use project owner for feed author
 		      "<author><name>$owner</name></author>\n";
 		if (defined $favicon) {
-			print "<icon>" . esc_url($favicon) . "</icon>\n";
+			print "<icon>" . esc_url_base($favicon) . "</icon>\n";
 		}
 		if (defined $logo) {
 			# not twice as wide as tall: 72 x 27 pixels
-			print "<logo>" . esc_url($logo) . "</logo>\n";
+			print "<logo>" . esc_url_base($logo) . "</logo>\n";
 		}
 		if (! %latest_date) {
 			# dummy date to keep the feed valid until commits trickle in:
-- 
2.21.3

[PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

using a base tag has the side-effect of not just changing the few URLs
of gitweb's static resources, but all other relative links (e.g. those
in a README.html), too.

Signed-off-by: Tobias Girstmair <tobi@isticktoit.net>
---
Apologies; missed a typo.

	tobias

 gitweb/gitweb.perl | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0959a78..f426060 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1616,6 +1616,19 @@ sub esc_url {
 	return $str;
 }
 
+# the stylesheet, favicon etc urls won't work correctly with path_info
+# unless we set the appropriate base URL. not using a <base> tag to not
+# also change relative URLs inserted by the user.
+sub esc_url_base {
+	my $url = shift;
+	my $prefix = $ENV{'PATH_INFO'}? esc_url($base_url)."/" : "";
+	if ($url !~ m{^(https?:)?//?}) {
+		return $prefix . esc_url($url);
+	} else {
+		return esc_url($url);
+	}
+}
+
 # quote unsafe characters in HTML attributes
 sub esc_attr {
 
@@ -2232,7 +2245,7 @@ sub git_get_avatar {
 		return $pre_white .
 		       "<img width=\"$size\" " .
 		            "class=\"avatar\" " .
-		            "src=\"".esc_url($url)."\" " .
+		            "src=\"".esc_url_base($url)."\" " .
 			    "alt=\"\" " .
 		       "/>" . $post_white;
 	} else {
@@ -4099,17 +4112,17 @@ sub print_header_links {
 	# print out each stylesheet that exist, providing backwards capability
 	# for those people who defined $stylesheet in a config file
 	if (defined $stylesheet) {
-		print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
+		print '<link rel="stylesheet" type="text/css" href="'.esc_url_base($stylesheet).'"/>'."\n";
 	} else {
 		foreach my $stylesheet (@stylesheets) {
 			next unless $stylesheet;
-			print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
+			print '<link rel="stylesheet" type="text/css" href="'.esc_url_base($stylesheet).'"/>'."\n";
 		}
 	}
 	print_feed_meta()
 		if ($status eq '200 OK');
 	if (defined $favicon) {
-		print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
+		print qq(<link rel="shortcut icon" href=").esc_url_base($favicon).qq(" type="image/png" />\n);
 	}
 }
 
@@ -4212,11 +4225,6 @@ sub git_header_html {
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
 EOF
-	# the stylesheet, favicon etc urls won't work correctly with path_info
-	# unless we set the appropriate base URL
-	if ($ENV{'PATH_INFO'}) {
-		print "<base href=\"".esc_url($base_url)."\" />\n";
-	}
 	print_header_links($status);
 
 	if (defined $site_html_head_string) {
@@ -4234,7 +4242,7 @@ sub git_header_html {
 	if (defined $logo) {
 		print $cgi->a({-href => esc_url($logo_url),
 		               -title => $logo_label},
-		              $cgi->img({-src => esc_url($logo),
+		              $cgi->img({-src => esc_url_base($logo),
 		                         -width => 72, -height => 27,
 		                         -alt => "git",
 		                         -class => "logo"}));
@@ -4299,7 +4307,7 @@ sub git_footer_html {
 		insert_file($site_footer);
 	}
 
-	print qq!<script type="text/javascript" src="!.esc_url($javascript).qq!"></script>\n!;
+	print qq!<script type="text/javascript" src="!.esc_url_base($javascript).qq!"></script>\n!;
 	if (defined $action &&
 	    $action eq 'blame_incremental') {
 		print qq!<script type="text/javascript">\n!.
@@ -8273,7 +8281,7 @@ sub git_feed {
 		if (defined $logo || defined $favicon) {
 			# prefer the logo to the favicon, since RSS
 			# doesn't allow both
-			my $img = esc_url($logo || $favicon);
+			my $img = esc_url_base($logo || $favicon);
 			print "<image>\n" .
 			      "<url>$img</url>\n" .
 			      "<title>$title</title>\n" .
@@ -8299,11 +8307,11 @@ sub git_feed {
 		      # use project owner for feed author
 		      "<author><name>$owner</name></author>\n";
 		if (defined $favicon) {
-			print "<icon>" . esc_url($favicon) . "</icon>\n";
+			print "<icon>" . esc_url_base($favicon) . "</icon>\n";
 		}
 		if (defined $logo) {
 			# not twice as wide as tall: 72 x 27 pixels
-			print "<logo>" . esc_url($logo) . "</logo>\n";
+			print "<logo>" . esc_url_base($logo) . "</logo>\n";
 		}
 		if (! %latest_date) {
 			# dummy date to keep the feed valid until commits trickle in:
-- 
2.21.3

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Junio C Hamano]

Tobias Girstmair <tobi@isticktoit.net> writes:

> using a base tag has the side-effect of not just changing the few URLs
> of gitweb's static resources, but all other relative links (e.g. those
> in a README.html), too.

Sorry, but I am not sure the description is understandable to the
intended readers of this sentence.

Where does this README.html come from?  

Is it stored in the history of the repository as a blob, and sent to
the browser with a call to git_blob_plain() sub?  Wouldn't that
codepath send the untrusted end-user data as an attachment, in which
case relative links in the blob do not get resolved relative to the
base URL anyway, no?

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

On Sun, Jul 12, 2020 at 02:00:01PM -0700, Junio C Hamano wrote:
>Sorry, but I am not sure the description is understandable to the
>intended readers of this sentence.
>
>Where does this README.html come from?

gitweb reads a README.html from each repository to display on the 
summary page. 'man 1 gitweb' has a paragraph on it under "Per-repository 
gitweb configuration".

>Is it stored in the history of the repository as a blob, and sent to
>the browser with a call to git_blob_plain() sub?  Wouldn't that

No, it's a plain file in a bare repository, placed there either manually 
or by a post-update hook.

>codepath send the untrusted end-user data as an attachment, in which
>case relative links in the blob do not get resolved relative to the
>base URL anyway, no?

I'm not exactly sure what you're saying. gitweb includes the README.html 
as-is (i.e. without escaping). If the user wanted to include an image, 
they'd write <img src="blob_plain/HEAD:/image.png"> (assuming this patch 
landed). In practise, these URLs will be rewritten by the 
markdown-to-html converter.

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Junio C Hamano]

Tobias Girstmair <tobi@isticktoit.net> writes:

> On Sun, Jul 12, 2020 at 02:00:01PM -0700, Junio C Hamano wrote:
>>Sorry, but I am not sure the description is understandable to the
>>intended readers of this sentence.
>>
>>Where does this README.html come from?
>
> gitweb reads a README.html from each repository to display on the
> summary page. 'man 1 gitweb' has a paragraph on it under
> "Per-repository gitweb configuration".
>
>>Is it stored in the history of the repository as a blob, and sent to
>>the browser with a call to git_blob_plain() sub?  Wouldn't that
>
> No, it's a plain file in a bare repository, placed there either
> manually or by a post-update hook.

OK.

> ... If the user wanted to
> include an image, they'd write <img src="blob_plain/HEAD:/image.png">
> (assuming this patch landed).

And without this patch, the src URL needs to know where this
repository appears in the site's URL namespace?

If that is the case, the change makes quite a lot of sense.

Thanks.

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

On Sun, Jul 12, 2020 at 09:34:36PM -0700, Junio C Hamano wrote:
>And without this patch, the src URL needs to know where this
>repository appears in the site's URL namespace?

Exactly. Sorry if my lack of proper terminology made this confusing

>If that is the case, the change makes quite a lot of sense.
>
>Thanks.
>

	tobias

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Junio C Hamano]

Tobias Girstmair <tobi@isticktoit.net> writes:

> On Sun, Jul 12, 2020 at 09:34:36PM -0700, Junio C Hamano wrote:
>>And without this patch, the src URL needs to know where this
>>repository appears in the site's URL namespace?
>
> Exactly. Sorry if my lack of proper terminology made this confusing

No, I was confused because I did not know Gitweb showed README.html
file in the $GIT_DIR of each repository.  Any other cases that are
affected by this, or is README the only one?

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

On Mon, Jul 13, 2020 at 07:44:45AM -0700, Junio C Hamano wrote:
>Any other cases that are
>affected by this, or is README the only one?
>

README.html is the only per-repository file, but gitweb has some 
(unset/nonexisting by default) config variables to include more HTML: 
$site_header, $site_footer, $home_text. These are described in 
'man 5 gitweb.conf' under "Changing gitweb's look". grepping for 
'insert_file' reveals them in the code.

	tobias

Re: [PATCH v3] gitweb: Replace <base> tag with full URLs (when using PATH_INFO)

[Tobias Girstmair]

On 13/07/2020 06:34, Junio C Hamano wrote:
[...]
> And without this patch, the src URL needs to know where this
> repository appears in the site's URL namespace?
> 
> If that is the case, the change makes quite a lot of sense.

Hi, this patch probably fell through the cracks. Can it please be 
considered for merging again?

As a reminder, here's a link to the patch:
   https://lore.kernel.org/git/20200712183329.3358-1-tobi@isticktoit.net/

Thanks,
	tobias