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

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

[Luben Tuikov]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

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(-)

[-- Attachment #2: 1207600725-p1.txt --]
[-- Type: text/plain, Size: 1893 bytes --]

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

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

[Petr Baudis]

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?

> +	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*',/((..)*)$/)

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

[Luben Tuikov]

--- Petr Baudis <pasky@suse.cz> wrote:
> 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

> 
> > +	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*',/((..)*)$/)
> 

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

[Jakub Narebski]

Luben Tuikov wrote:

> +# 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

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

[Junio C Hamano]

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;
>> > +	$str =~ s/\r/\\r/g;
>> 
>> \r? Not \n?
>
> Yes, \r, not \n.

\r to \\r? Not to \\\r?

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

[Jakub Narebski]

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".

Otherwise we could use simplier
        $str =~ s/([\\"\r])/\\\1/g;

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

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

[Junio C Hamano]

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?

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 '_'.

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

[Petr Baudis]

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.

-- 
				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*',/((..)*)$/)

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

[Jakub Narebski]

Petr Baudis wrote:

> 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

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

[Luben Tuikov]

--- Junio C Hamano <junkio@cox.net> wrote:
> 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.

> 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