threads / patch / 5840

patchgitweb: Convert Content-Disposition filenames into qtext

Subject: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

## tl;dr

10 messages between Oct 6, 2006 and Oct 7, 2006. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Luben Tuikov· Oct 6, 2006, 19:18 UTC · lore

Convert a string (e.g. a filename) into qtext as defined in RFC 822, from RFC 2183. To be used by Content-Disposition.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.perl |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)
Show changes to gitweb/gitweb.perl +14 −4
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f848648..a35d02c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -520,6 +520,16 @@ sub esc_html {
 	return $str;
 }
 
+# Convert a string (e.g. a filename) into qtext as defined
+# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
+sub to_qtext {
+	my $str = shift;
+	$str =~ s/\\/\\\\/g;
+	$str =~ s/\"/\\\"/g;
+	$str =~ s/\r/\\r/g;
+	return $str;
+}
+
 # git may return quoted and escaped filenames
 sub unquote {
 	my $str = shift;
@@ -2742,7 +2752,7 @@ sub git_blob_plain {
 	print $cgi->header(
 		-type => "$type",
 		-expires=>$expires,
-		-content_disposition => 'inline; filename="' . "$save_as" . '"');
+		-content_disposition => 'inline; filename="' . to_qtext("$save_as") . '"');
 	undef $/;
 	binmode STDOUT, ':raw';
 	print <$fd>;
@@ -2917,7 +2927,7 @@ sub git_snapshot {
 	print $cgi->header(
 		-type => 'application/x-tar',
 		-content_encoding => $ctype,
-		-content_disposition => 'inline; filename="' . "$filename" . '"',
+		-content_disposition => 'inline; filename="' . to_qtext("$filename") . '"',
 		-status => '200 OK');
 
 	my $git = git_cmd_str();
@@ -3224,7 +3234,7 @@ sub git_blobdiff {
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
-			-content_disposition => 'inline; filename="' . "$file_name" . '.patch"');
+			-content_disposition => 'inline; filename="' . to_qtext("$file_name") . '.patch"');
 
 		print "X-Git-Url: " . $cgi->self_url() . "\n\n";
 
@@ -3327,7 +3337,7 @@ sub git_commitdiff {
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
-			-content_disposition => 'inline; filename="' . "$filename" . '"');
+			-content_disposition => 'inline; filename="' . to_qtext("$filename") . '"');
 		my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
 		print <<TEXT;
 From: $co{'author'}

-- 
1.4.2.3.g0954
Petr Baudis· Oct 6, 2006, 19:20 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Dear diary, on Fri, Oct 06, 2006 at 09:18:01PM CEST, I got a letter where Luben Tuikov <ltuikov@yahoo.com> said that...

Show 7 quoted lines
> Convert a string (e.g. a filename) into qtext as defined
> in RFC 822, from RFC 2183.  To be used by Content-Disposition.
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  gitweb/gitweb.perl |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)
Content-Description: 1207600725-p1.txt
Show 15 quoted lines
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f848648..a35d02c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -520,6 +520,16 @@ sub esc_html {
>  	return $str;
>  }
>  
> +# Convert a string (e.g. a filename) into qtext as defined
> +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
> +sub to_qtext {
> +	my $str = shift;
> +	$str =~ s/\\/\\\\/g;
> +	$str =~ s/\"/\\\"/g;
> +	$str =~ s/\r/\\r/g;
\r? Not \n?
Show 6 quoted lines
> +	return $str;
> +}
> +
>  # git may return quoted and escaped filenames
>  sub unquote {
>  	my $str = shift;
Other than that,
Acked-by: Petr Baudis <pasky@suse.cz>
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Luben Tuikov· Oct 6, 2006, 19:30 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

--- Petr Baudis <pasky@suse.cz> wrote:
Show 28 quoted lines
> Dear diary, on Fri, Oct 06, 2006 at 09:18:01PM CEST, I got a letter
> where Luben Tuikov <ltuikov@yahoo.com> said that...
> > Convert a string (e.g. a filename) into qtext as defined
> > in RFC 822, from RFC 2183.  To be used by Content-Disposition.
> > 
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> > ---
> >  gitweb/gitweb.perl |   18 ++++++++++++++----
> >  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> Content-Description: 1207600725-p1.txt
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index f848648..a35d02c 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -520,6 +520,16 @@ sub esc_html {
> >  	return $str;
> >  }
> >  
> > +# Convert a string (e.g. a filename) into qtext as defined
> > +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
> > +sub to_qtext {
> > +	my $str = shift;
> > +	$str =~ s/\\/\\\\/g;
> > +	$str =~ s/\"/\\\"/g;
> > +	$str =~ s/\r/\\r/g;
> 
> \r? Not \n?
Yes, \r, not \n.
\n is LF, \r is CR, from ASCII(7).

LF is legal in qtext as defined in RFC 822. The illegals in qtext are CR, backslash and double quote.

   Luben
Show 19 quoted lines
> 
> > +	return $str;
> > +}
> > +
> >  # git may return quoted and escaped filenames
> >  sub unquote {
> >  	my $str = shift;
> 
> Other than that,
> 
> Acked-by: Petr Baudis <pasky@suse.cz>
> 
> -- 
> 				Petr "Pasky" Baudis
> Stuff: http://pasky.or.cz/
> #!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
> $/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
> lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
>
Junio C Hamano· Oct 7, 2006, 09:46 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 11 quoted lines
>> > +# Convert a string (e.g. a filename) into qtext as defined
>> > +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
>> > +sub to_qtext {
>> > +	my $str = shift;
>> > +	$str =~ s/\\/\\\\/g;
>> > +	$str =~ s/\"/\\\"/g;
>> > +	$str =~ s/\r/\\r/g;
>> 
>> \r? Not \n?
>
> Yes, \r, not \n.
\r to \\r? Not to \\\r?
Jakub Narebski· Oct 7, 2006, 10:06 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Junio C Hamano wrote:
Show 8 quoted lines
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
>>>> +# Convert a string (e.g. a filename) into qtext as defined
>>>> +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
>>>> +sub to_qtext {
>>>> +  my $str = shift;
>>>> +  $str =~ s/\\/\\\\/g;
>>>> +  $str =~ s/\"/\\\"/g;
Here probably it could be
        $str =~ s/"/\\"/g;
Show 7 quoted lines
>>>> +  $str =~ s/\r/\\r/g;
>>> 
>>> \r? Not \n?
>>
>> Yes, \r, not \n.
> 
> \r to \\r? Not to \\\r?

We want "\r" in suggested filename, not "\ " I think, so it is "\\r".

Otherwise we could use simplier
        $str =~ s/([\\"\r])/\\\1/g;
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Junio C Hamano· Oct 7, 2006, 10:34 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Jakub Narebski <jnareb@gmail.com> writes:
Show 24 quoted lines
> Junio C Hamano wrote:
>
>> Luben Tuikov <ltuikov@yahoo.com> writes:
>> 
>>>>> +# Convert a string (e.g. a filename) into qtext as defined
>>>>> +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
>>>>> +sub to_qtext {
>>>>> +  my $str = shift;
>>>>> +  $str =~ s/\\/\\\\/g;
>>>>> +  $str =~ s/\"/\\\"/g;
>
> Here probably it could be
>         $str =~ s/"/\\"/g;
>
>>>>> +  $str =~ s/\r/\\r/g;
>>>> 
>>>> \r? Not \n?
>>>
>>> Yes, \r, not \n.
>> 
>> \r to \\r? Not to \\\r?
>
> We want "\r" in suggested filename, not "\
> " I think, so it is "\\r".
Is that what you guys are attempting to achieve?

If we are trying to suggest a filename that is safe by avoiding certain characters, I suspect leaving a backslash and dq as-is is just as bad as leaving a CR in. So if that is the goal here, I think it might be better and a lot simpler to just replace each run of bytes not in Portable Filename Character Set with an underscore '_'.

Luben Tuikov· Oct 7, 2006, 18:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

--- Junio C Hamano <junkio@cox.net> wrote:
Show 28 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >
> >> Luben Tuikov <ltuikov@yahoo.com> writes:
> >> 
> >>>>> +# Convert a string (e.g. a filename) into qtext as defined
> >>>>> +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
> >>>>> +sub to_qtext {
> >>>>> +  my $str = shift;
> >>>>> +  $str =~ s/\\/\\\\/g;
> >>>>> +  $str =~ s/\"/\\\"/g;
> >
> > Here probably it could be
> >         $str =~ s/"/\\"/g;
> >
> >>>>> +  $str =~ s/\r/\\r/g;
> >>>> 
> >>>> \r? Not \n?
> >>>
> >>> Yes, \r, not \n.
> >> 
> >> \r to \\r? Not to \\\r?
> >
> > We want "\r" in suggested filename, not "\
> > " I think, so it is "\\r".
> 
> Is that what you guys are attempting to achieve?
I think so.
Show 6 quoted lines
> If we are trying to suggest a filename that is safe by avoiding
> certain characters, I suspect leaving a backslash and dq as-is
> is just as bad as leaving a CR in.  So if that is the goal here,
> I think it might be better and a lot simpler to just replace
> each run of bytes not in Portable Filename Character Set with an
> underscore '_'.

I think that if I were to download a file which had those chars in it, I'd like to at least be able to see the _intention_ of what chars the actual file name had.

So if I download a filename which looks like this:
     This is a \" test \" file \\.\r
Then I know that the intention had been:
     This is a " test " file \.<CR>

It becomes an intention, since it needs to be carried over a qtext.

   Luben
Petr Baudis· Oct 7, 2006, 11:46 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Dear diary, on Sat, Oct 07, 2006 at 12:06:31PM CEST, I got a letter where Jakub Narebski <jnareb@gmail.com> said that...

Show 10 quoted lines
> >>>> +  $str =~ s/\r/\\r/g;
> >>> 
> >>> \r? Not \n?
> >>
> >> Yes, \r, not \n.
> > 
> > \r to \\r? Not to \\\r?
> 
> We want "\r" in suggested filename, not "\
> " I think, so it is "\\r".

Oh, yes. Lubin wants. It looked sane until I've read it as you explicitly wrote it. ;-)

That's "obviously" wrong. In qtext, \r means just r, no special interpretation is done. So we indeed _would_ want "\ ". Which is of course a nice trap for buggy browsers so in fact we obviously do not want that. I think it's not wort the potential problems to try to carry newlines in the header, so I would just replace that line with

	$str =~ s/[\n\r]/_/g;
as per Junio's suggestion.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Jakub Narebski· Oct 7, 2006, 12:11 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Petr Baudis wrote:
Show 26 quoted lines
> Dear diary, on Sat, Oct 07, 2006 at 12:06:31PM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...
>> >>>> +  $str =~ s/\r/\\r/g;
>> >>> 
>> >>> \r? Not \n?
>> >>
>> >> Yes, \r, not \n.
>> > 
>> > \r to \\r? Not to \\\r?
>> 
>> We want "\r" in suggested filename, not "\
>> " I think, so it is "\\r".
> 
> Oh, yes. Lubin wants. It looked sane until I've read it as you
> explicitly wrote it. ;-)
> 
> That's "obviously" wrong. In qtext, \r means just r, no special
> interpretation is done. So we indeed _would_ want "\
> ". Which is of course a nice trap for buggy browsers so in fact we
> obviously do not want that. I think it's not wort the potential problems
> to try to carry newlines in the header, so I would just replace that
> line with
> 
>       $str =~ s/[\n\r]/_/g;
> 
> as per Junio's suggestion.
Bu the way, using the following script:

-- >8 -- #!/usr/bin/perl

use strict; use warnings; use CGI qw(:standard :escapeHTML -nosticky);

binmode STDOUT, ':utf8';
our $cgi = new CGI;
print $cgi->header(
        -type => 'text/plain',
        -charset => 'utf-8',
        -content_disposition => 'inline; filename="test\".\\"test\\n.\\\n"');

print "TEST\n"; -- >8 --

I've checked that at least Mozilla 1.7.12 wants to using "\n" in file name instead of literal eoln.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Jakub Narebski· Oct 7, 2006, 09:05 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: Convert Content-Disposition filenames into qtext

Luben Tuikov wrote:
Show 9 quoted lines
> +# Convert a string (e.g. a filename) into qtext as defined
> +# in RFC 822, from RFC 2183.  To be used by Content-Disposition.
> +sub to_qtext {
> +       my $str = shift;
> +       $str =~ s/\\/\\\\/g;
> +       $str =~ s/\"/\\\"/g;
> +       $str =~ s/\r/\\r/g;
> +       return $str;
> +}
I'd rather add \n too.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

← back to recent threads