threads / patch / 5581

patchgitweb: use correct mime type even if filename has multiple dots.

Subject: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

## tl;dr

10 messages between Sep 16, 2006 and Sep 19, 2006. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Martin Waitz· Sep 16, 2006, 21:09 UTC · lore

Match the last part of the filename agains the extention from the mime database instead of insisting that it starts at the first dot.

Signed-off-by: Martin Waitz <tali@admingilde.org>
---
 gitweb/gitweb.perl |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
Show changes to gitweb/gitweb.perl +5 −4
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ee561c6..7501251 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1106,7 +1106,6 @@ sub mimetype_guess_file {
 	my $mimemap = shift;
 	-r $mimemap or return undef;
 
-	my %mimemap;
 	open(MIME, $mimemap) or return undef;
 	while (<MIME>) {
 		next if m/^#/; # skip comments
@@ -1114,14 +1113,16 @@ sub mimetype_guess_file {
 		if (defined $exts) {
 			my @exts = split(/\s+/, $exts);
 			foreach my $ext (@exts) {
-				$mimemap{$ext} = $mime;
+				if ($filename =~ /\.$ext$/) {
+					close(MIME);
+					return $mime;
+				}
 			}
 		}
 	}
 	close(MIME);
 
-	$filename =~ /\.(.*?)$/;
-	return $mimemap{$1};
+	return undef;
 }
 
 sub mimetype_guess {
-- 
1.4.2.gb8b6b

-- 
Martin Waitz
Jakub Narebski· Sep 16, 2006, 21:44 UTC · re: Martin Waitz · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

Martin Waitz wrote:
> Match the last part of the filename agains the extention from the
> mime database instead of insisting that it starts at the first dot.
[...]
> -     $filename =~ /\.(.*?)$/;
> -     return $mimemap{$1};

Actually, that is non-greedy match, so the above code insist that extension starts at the _last_ dot.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Martin Waitz· Sep 17, 2006, 07:51 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

hoi :)
On Sat, Sep 16, 2006 at 11:44:52PM +0200, Jakub Narebski wrote:
Show 10 quoted lines
> Martin Waitz wrote:
> 
> > Match the last part of the filename agains the extention from the
> > mime database instead of insisting that it starts at the first dot.
> [...]
> > -     $filename =~ /\.(.*?)$/;
> > -     return $mimemap{$1};
> 
> Actually, that is non-greedy match, so the above code insist that 
> extension starts at the _last_ dot.

hmm, but it didn't work for me. I had filenames like "man/program.8.html" which got served as "text/html" with the old code.

Besides, the new code would cope with extentions that contain a dot itself.

Looking at /etc/mime.types, it only contains pcf.Z but perhaps it should also contain tar.gz or similiar.

Show 11 quoted lines
> 
> -- 
> Jakub Narebski
> Warsaw, Poland
> ShadeHawk on #git
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Martin Waitz
Jakub Narebski· Sep 17, 2006, 08:23 UTC · re: Martin Waitz · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

Martin Waitz wrote:
Show 17 quoted lines
> hoi :)
> 
> On Sat, Sep 16, 2006 at 11:44:52PM +0200, Jakub Narebski wrote:
>> Martin Waitz wrote:
>> 
>> > Match the last part of the filename agains the extention from the
>> > mime database instead of insisting that it starts at the first dot.
>> [...]
>> > -     $filename =~ /\.(.*?)$/;
>> > -     return $mimemap{$1};
>> 
>> Actually, that is non-greedy match, so the above code insist that 
>> extension starts at the _last_ dot.
> 
> hmm, but it didn't work for me.
> I had filenames like "man/program.8.html" which got served as
> "text/html" with the old code.

And why it shouldn't? From the extension it is HTML page, I would guess manpage converted to HTML (pretty-printed manpage). And it should be served with text/html mimetype.

> Besides, the new code would cope with extentions that contain a
> dot itself.
But is unnecessary complicated, and I guess performance suffers a bit.
> Looking at /etc/mime.types, it only contains pcf.Z but perhaps
So the *.pcf.Z file wouldn't get correct mimetype. No big.
> it should also contain tar.gz or similiar.
You can't properly serve tar.gz as something other than with
application/x-gzip mimetype. If you want to serve it as/with 
application/x-tar mimetype, you would need to add 
        Content-Encoding: x-gzip
in addition to
        Content-Type: application/x-tar
And your code doesn't do that.
The _last_ extension defines the type.

Besides, with the exception of files which can be displayed in browser, i.e. HTML files and images it doesn't matter what the mimetype is, if only binary files get binary mimetype (e.g. generic application/octet-stream).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Martin Waitz· Sep 19, 2006, 08:29 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

On Sun, Sep 17, 2006 at 10:23:45AM +0200, Jakub Narebski wrote:
Show 11 quoted lines
> Martin Waitz wrote:
> 
> > hoi :)
> > 
> > hmm, but it didn't work for me.
> > I had filenames like "man/program.8.html" which got served as
> > "text/html" with the old code.
> 
> And why it shouldn't? From the extension it is HTML page, I would guess
> manpage converted to HTML (pretty-printed manpage). And it should be served
> with text/html mimetype.
arg, typo, it got served as "text/plain".
-- 
Martin Waitz
Jakub Narebski· Sep 19, 2006, 11:57 UTC · re: Martin Waitz · lore

[PATCH] gitweb: Fix mimetype_guess_file for files with multiple extensions

Fix getting correct mimetype for "blob_plain" view for files which have multiple extensions, e.g. foo.1.html; now only the last extension is used to find mimetype.

Noticed by Martin Waitz.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is much simpler (and faster!) correction to the mentioned problem.
I just don't grok regular expressions, not completly.
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to gitweb/gitweb.perl +1 −1
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 01fae94..034cdf1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1199,7 +1199,7 @@ sub mimetype_guess_file {
 	}
 	close(MIME);
 
-	$filename =~ /\.(.*?)$/;
+	$filename =~ /\.([^.]*)$/;
 	return $mimemap{$1};
 }
 
-- 
1.4.2.1
Junio C Hamano· Sep 17, 2006, 08:41 UTC · re: Martin Waitz · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

Martin Waitz <tali@admingilde.org> writes:
Show 9 quoted lines
>> > -     $filename =~ /\.(.*?)$/;
>> > -     return $mimemap{$1};
>> 
>> Actually, that is non-greedy match, so the above code insist that 
>> extension starts at the _last_ dot.
>
> hmm, but it didn't work for me.
> I had filenames like "man/program.8.html" which got served as
> "text/html" with the old code.

It based its decision on '.html' part, which is expected from non-greedy match (if I were writing the pattern, I would have written /\.[^.]+$/ instead, though). Are you trying to have it behave differently between "x.8.html" and "x.html"?

> Looking at /etc/mime.types, it only contains pcf.Z but perhaps
> it should also contain tar.gz or similiar.
Probably.  But that makes me think it might be better to:
 - read in mime.types, sort the entries with length of the
   suffixes (longer first);
 - try matching the suffixes from longer to shorter and pick the
   first match.

Without that, you would not be able to cope with a /etc/mime.types that looks like this, no?

        application/a	a
        application/b	b.a
Perhaps something like the attached.
---
Show changes to gitweb/gitweb.perl +30 −11
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a81c8d4..4994a33 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1140,23 +1140,42 @@ sub mimetype_guess_file {
 	my $filename = shift;
 	my $mimemap = shift;
 	-r $mimemap or return undef;
+	local($_);
 
-	my %mimemap;
-	open(MIME, $mimemap) or return undef;
+	open MIME, $mimemap
+	    or return undef;
+
+	# Under mod_perl caching this may make a lot of sense...
+	my @mime = ();
+	my $maxlen = 0;
 	while (<MIME>) {
-		next if m/^#/; # skip comments
-		my ($mime, $exts) = split(/\t+/);
-		if (defined $exts) {
-			my @exts = split(/\s+/, $exts);
-			foreach my $ext (@exts) {
-				$mimemap{$ext} = $mime;
+		next if /^#/;
+		chomp;
+		my ($mimetype, @ext) = split(/\s+/);
+		for (@ext) {
+			my $len = length;
+			my $map = $mime[$len];
+			if (!$map) {
+				$mime[$len] = $map = {};
+				$maxlen = $len if ($maxlen < $len);
 			}
+			# We could detect duplicate definition here... i.e.
+			# onetype	ext
+			# anothertype	ext
+			$map->{$_} = $mimetype;
 		}
 	}
-	close(MIME);
+	close MIME;
 
-	$filename =~ /\.(.*?)$/;
-	return $mimemap{$1};
+	for ($filename) {
+		for (my $len = $maxlen; 0 < $len; $len--) {
+			my $map = $mime[$len];
+			while (my ($ext, $type) = each %$map) {
+				return $type if (/\.\Q$ext\E$/);
+			}
+		}
+	}
+	return undef;
 }
 
 sub mimetype_guess {
Jakub Narebski· Sep 17, 2006, 08:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

Junio C Hamano wrote:
Show 18 quoted lines
>> Looking at /etc/mime.types, it only contains pcf.Z but perhaps
>> it should also contain tar.gz or similiar.
> 
> Probably.  But that makes me think it might be better to:
> 
>  - read in mime.types, sort the entries with length of the
>    suffixes (longer first);
> 
>  - try matching the suffixes from longer to shorter and pick the
>    first match.
> 
> Without that, you would not be able to cope with a /etc/mime.types
> that looks like this, no?
> 
>         application/a   a
>         application/b   b.a
> 
> Perhaps something like the attached.

Is it really useful? Usually the suffix in mime.types doesn't contain dot itself. Besides, to be said we need correct mimetype only for files which can be displayed in browser (HTML, XHTML, images: png, gif, jpeg, perhaps XML). All other can get generic mimetype, i.e. application/octet-stream for binary files (to be saved) and text/plain for text file (to be displayer as-is in browser).

Besides, performance will suffer for "blob_plain" view. One hash lookup vs. nested loops.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Junio C Hamano· Sep 17, 2006, 09:07 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

Jakub Narebski <jnareb@gmail.com> writes:
Show 6 quoted lines
> Is it really useful? Usually the suffix in mime.types doesn't contain dot
> itself. Besides, to be said we need correct mimetype only for files which
> can be displayed in browser (HTML, XHTML, images: png, gif, jpeg, perhaps
> XML). All other can get generic mimetype, i.e. application/octet-stream for
> binary files (to be saved) and text/plain for text file (to be displayer
> as-is in browser).

Sorry, I do not think that is quite correct. The browser can launch an appropriate application on the downloaded file, as long as you send the data labelled with proper mimetype. If you send out application/octet-stream, of course that would not work.

Having said that, I am not sure what special things Martin wanted to do with x.8.html that is different from y.html, so maybe discussing the patch does not have merit at all.

Martin Waitz· Sep 19, 2006, 09:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: use correct mime type even if filename has multiple dots.

hoi :)
On Sun, Sep 17, 2006 at 01:41:40AM -0700, Junio C Hamano wrote:
Show 13 quoted lines
>  - read in mime.types, sort the entries with length of the
>    suffixes (longer first);
> 
>  - try matching the suffixes from longer to shorter and pick the
>    first match.
> 
> Without that, you would not be able to cope with a /etc/mime.types
> that looks like this, no?
> 
>         application/a	a
>         application/b	b.a
> 
> Perhaps something like the attached.
works perfectly, thanks.
-- 
Martin Waitz

← back to recent threads