Re: [PATCH 13/18] gitweb: Add commented url & url hash to page footer

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 10, 2010, 00:26 UTC
Message-ID: <m38vzy1nkl.fsf@localhost.localdomain>
In-Reply-To: <1291931844-28454-14-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

Show 7 quoted lines

> This is mostly a debugging tool, but it adds a small bit of information
> to the footer:
> 
> <!--
> 	Full URL: |http://localhost/gitweb-caching/gitweb.cgi?p=/project.git;a=summary|
> 	URL Hash: |7a31cfb8a43f5643679eec88aa9d7981|
> -->

Nice idea. It helps with debugging and doesn't introduce information leakage.

Note that in my rewrite there would be *three* pieces of information, not two. Namely:

  Full URL: |http://localhost/gitweb-caching/gitweb.cgi/project.git|
  Key:      |http://localhost/gitweb-caching/gitweb.cgi?p=/project.git;a=summary|
  Key hash: |7a31cfb8a43f5643679eec88aa9d7981|

Show 5 quoted lines

> 
> The first bit tells you what the url that generated the page actually was, the second is
> the hash used to store the file with the first two characters being used as the directory:
> 
> <cachedir>/7a/31cfb8a43f5643679eec88aa9d7981

Isn't it

  <cachedir>/7a/7a31cfb8a43f5643679eec88aa9d7981

in your series?

> 
> Also useful for greping through the existing cache and finding files with unique paths that
> you may want to explicitly flush.

Though probably better 'cache_admin' page would be ultimately best solution, see proof of concept in

  [RFC PATCHv6 24/24] gitweb: Add beginnings of cache administration page (proof of concept)
  http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163051
  http://repo.or.cz/w/git/jnareb-git.git/commitdiff/aa9fd77ff206eae8838fdde626d2afea563f9f75

Show 29 quoted lines

> 
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
>  gitweb/gitweb.perl  |    7 +++++++
>  gitweb/lib/cache.pl |    4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e8c028b..7f8292e 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -303,6 +303,9 @@ our $fullhashpath = *STDOUT;
>  our $fullhashbinpath = *STDOUT;
>  our $fullhashbinpathfinal = *STDOUT;
>  
> +our $full_url;
> +our $urlhash;
> +
>  # configuration for 'highlight' (http://www.andre-simon.de/)
>  # match by basename
>  our %highlight_basename = (
> @@ -3663,6 +3666,10 @@ sub git_footer_html {
>  
>  	print "<div class=\"page_footer\">\n";
>  	print "<div class=\"cachetime\">Cache Last Updated: ". gmtime( time ) ." GMT</div>\n";
> +	print	"<!--\n".
> +		"	Full URL: |$full_url|\n".
> +		"	URL Hash: |$urlhash|\n".
> +		"-->\n" if ($cache_enable);

Don't you need to esc_html on it? $full_url can contain ' -->', and what you would do then?

Show 12 quoted lines

>  	if (defined $project) {
>  		my $descr = git_get_project_description($project);
>  		if (defined $descr) {
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index fafc028..63dbe9e 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -30,8 +30,8 @@ sub cache_fetch {
>  		print "Cache directory created successfully\n";
>  	}
>  
> -	our $full_url = "$my_url?". $ENV{'QUERY_STRING'};

Note that $my_url is $cgi->url(), which does not include path_info.

Show 8 quoted lines

> -	our $urlhash = md5_hex($full_url);
> +	$full_url = "$my_url?". $ENV{'QUERY_STRING'};
> +	$urlhash = md5_hex($full_url);
>  	our $fullhashdir = "$cachedir/". substr( $urlhash, 0, 2) ."/";
>  
>  	eval { mkpath( $fullhashdir, 0, 0777 ) };
> -- 
> 1.7.2.3

Looks quite nice.

-- 
Jakub Narebski
Poland
ShadeHawk on #git