threads / discuss / 11118

Fix UTF Encoding issue

Subject: Fix UTF Encoding issue

## tl;dr

24 messages between Dec 3, 2007 and Dec 4, 2007.

replies: 23people: 6as markdown or json

Benjamin Close· Dec 3, 2007, 10:02 UTC · lore
>From 83042abf3967b455953cddeab43e33c1d59c6f03 Mon Sep 17 00:00:00 2001
From: Benjamin Close <Benjamin.Close@clearchain.com>
Date: Sun, 2 Dec 2007 15:09:00 -0800
Subject: [PATCH] Gitweb: Fix encoding to always translate rather than 
sometimes fail

When performing the utf translation don't test if $res is defined. It appears that it is defined even when the conversion fails. This causes failures on the writing of the output stream which is expecting UTF.

Instead, immediately return if conversion is successful else force
the translation to the fallback encoding
---
  gitweb/gitweb.perl |    8 ++------
  1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 491a3f4..00bbcdf 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -696,12 +696,8 @@ sub validate_refname {
  sub to_utf8 {
  	my $str = shift;
  	my $res;
-	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
-	if (defined $res) {
-		return $res;
-	} else {
-		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
-	}
+	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
+	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
  }

  # quote unsafe chars, but keep the slash, even when it's not
-- 
1.5.3.6
Junio C Hamano· Dec 3, 2007, 10:14 UTC · re: Benjamin Close · lore

Re: Fix UTF Encoding issue

Benjamin Close <Benjamin.Close@clearchain.com> writes:
Show 22 quoted lines
>>From 83042abf3967b455953cddeab43e33c1d59c6f03 Mon Sep 17 00:00:00 2001
> From: Benjamin Close <Benjamin.Close@clearchain.com>
> Date: Sun, 2 Dec 2007 15:09:00 -0800
> Subject: [PATCH] Gitweb: Fix encoding to always translate rather than
> sometimes fail
>
> When performing the utf translation don't test if $res is defined.
> It appears that it is defined even when the conversion fails. This causes
> failures on the writing of the output stream which is expecting UTF.
> @@ -696,12 +696,8 @@ sub validate_refname {
>  sub to_utf8 {
>  	my $str = shift;
>  	my $res;
> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> -	if (defined $res) {
> -		return $res;
> -	} else {
> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> -	}
> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>  }
This is funny.

I thought the standard catch ... throw idiom in Perl was to do the above like this:

	my $res;
        eval { $res = decode_utf8($str, Encode::FB_CROAK); };
        if ($@) {
        	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
	}
	return $res;
(alternatively, you can assign return value of eval {} to $res).
Ismail Dönmez· Dec 3, 2007, 11:32 UTC · re: Junio C Hamano · lore

Re: Fix UTF Encoding issue

Monday 03 December 2007 Tarihinde 12:14:43 yazmıştı:
Show 36 quoted lines
> Benjamin Close <Benjamin.Close@clearchain.com> writes:
> >>From 83042abf3967b455953cddeab43e33c1d59c6f03 Mon Sep 17 00:00:00 2001
> >
> > From: Benjamin Close <Benjamin.Close@clearchain.com>
> > Date: Sun, 2 Dec 2007 15:09:00 -0800
> > Subject: [PATCH] Gitweb: Fix encoding to always translate rather than
> > sometimes fail
> >
> > When performing the utf translation don't test if $res is defined.
> > It appears that it is defined even when the conversion fails. This causes
> > failures on the writing of the output stream which is expecting UTF.
> > @@ -696,12 +696,8 @@ sub validate_refname {
> >  sub to_utf8 {
> >  	my $str = shift;
> >  	my $res;
> > -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> > -	if (defined $res) {
> > -		return $res;
> > -	} else {
> > -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> > -	}
> > +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
> > +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >  }
>
> This is funny.
>
> I thought the standard catch ... throw idiom in Perl was to do the above
> like this:
>
> 	my $res;
>         eval { $res = decode_utf8($str, Encode::FB_CROAK); };
>         if ($@) {
>         	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> 	}
> 	return $res;

I think this is correct, but the current code in gitweb doesn't look correct since it checks for $res and not $@.

Regards, ismail

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Jakub Narebski· Dec 3, 2007, 12:06 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

Ismail Dönmez <ismail@pardus.org.tr> writes:
> Monday 03 December 2007 Tarihinde 12:14:43 yazmıştı:
>> Benjamin Close <Benjamin.Close@clearchain.com> writes:
Show 22 quoted lines
>>> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
>>> -	if (defined $res) {
>>> -		return $res;
>>> -	} else {
>>> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>>> -	}
>>> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
>>> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>>>  }
>>
>> I thought the standard catch ... throw idiom in Perl was to do the above
>> like this:
>>
>> 	my $res;
>>         eval { $res = decode_utf8($str, Encode::FB_CROAK); };
>>         if ($@) {
>>         	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>> 	}
>> 	return $res;
> 
> I think this is correct, but the current code in gitweb doesn't look correct 
> since it checks for $res and not $@.

First version of the patch was created by Martin Koegler. I have participated in creating the version which is now in gitweb, but I have to say that I wrote it based on decode_utf8 documentation... which doesn't necessarily agree with facts :-(

I'm all for the "throw idion" version. Ack.
-- 
Jakub Narebski
Martin Koegler· Dec 3, 2007, 16:38 UTC · re: Jakub Narebski · lore

Re: Fix UTF Encoding issue

On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
Show 12 quoted lines
> Ismail Dönmez <ismail@pardus.org.tr> writes:
> > Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
> >> Benjamin Close <Benjamin.Close@clearchain.com> writes:
> >>> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> >>> -	if (defined $res) {
> >>> -		return $res;
> >>> -	} else {
> >>> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >>> -	}
> >>> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
> >>> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >>>  }

This version is broken on Debian sarge and etch. Feeding a UTF-8 and a latin1 encoding of the same character sequence yields to different results.

Show 18 quoted lines
> >>
> >> I thought the standard catch ... throw idiom in Perl was to do the above
> >> like this:
> >>
> >> 	my $res;
> >>         eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> >>         if ($@) {
> >>         	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >> 	}
> >> 	return $res;
> > 
> > I think this is correct, but the current code in gitweb doesn't look correct 
> > since it checks for $res and not $@.
> 
> First version of the patch was created by Martin Koegler. I have
> participated in creating the version which is now in gitweb, but I
> have to say that I wrote it based on decode_utf8
> documentation... which doesn't necessarily agree with facts :-(
eval { $res = decode_utf8(...); }
if ($@) 
     return decode(...);
return $res
or
eval { $res = decode_utf8(...); }
if (defined $res)
      return $res;
else
    return decode(...);

show the same (wrong) behaviour on Debian sarge. They do not always decode non UTF-8 characters correctly, eg. #öäü does not work #äöüä does work

On Debian etch, both versions are working.
> I'm all for the "throw idion" version. Ack.
mfg Martin Kögler
Jakub Narebski· Dec 3, 2007, 17:02 UTC · re: Martin Koegler · lore

Re: Fix UTF Encoding issue

On Mon, 3 Dec 2007, Martin Koegler wrote:
Show 16 quoted lines
> On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
>> Ismail Dönmez <ismail@pardus.org.tr> writes:
>>> Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
>>>> Benjamin Close <Benjamin.Close@clearchain.com> writes:
>>>>> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
>>>>> -	if (defined $res) {
>>>>> -		return $res;
>>>>> -	} else {
>>>>> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>>>>> -	}
>>>>> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
>>>>> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>>>>>  }
> 
> This version is broken on Debian sarge and etch. Feeding a UTF-8 and a latin1
> encoding of the same character sequence yields to different results.
[...]
Show 19 quoted lines
> eval { $res = decode_utf8(...); }
> if ($@) 
>      return decode(...);
> return $res
> 
> or
> 
> eval { $res = decode_utf8(...); }
> if (defined $res)
>       return $res;
> else
>     return decode(...);
> 
> show the same (wrong) behaviour on Debian sarge. They do not always
> decode non UTF-8 characters correctly, eg.
> #öäü does not work
> #äöüä does work
> 
> On Debian etch, both versions are working.

I don't know enough Perl to decide if it is a bug in gitweb usage of decode_utf8, if it is a bug in your version of Encode, or if it is bug in Encode.

Send copy of this mail to maintainers of Encode perl module.
-- 
Jakub Narebski
Poland
Benjamin Close· Dec 3, 2007, 21:46 UTC · re: Jakub Narebski · lore

Re: Fix UTF Encoding issue

Jakub Narebski wrote:
Show 25 quoted lines
> On Mon, 3 Dec 2007, Martin Koegler wrote:
>   
>> On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
>>     
>>> Ismail Dönmez <ismail@pardus.org.tr> writes:
>>>       
>>>> Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
>>>>         
>>>>> Benjamin Close <Benjamin.Close@clearchain.com> writes:
>>>>>           
>>>>>> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
>>>>>> -	if (defined $res) {
>>>>>> -		return $res;
>>>>>> -	} else {
>>>>>> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>>>>>> -	}
>>>>>> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
>>>>>> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
>>>>>>  }
>>>>>>             
>> This version is broken on Debian sarge and etch. Feeding a UTF-8 and a latin1
>> encoding of the same character sequence yields to different results.
>>     
>
>   
For the record, this was on a debian sid machine.

#perl --version This is perl, v5.8.8 built for x86_64-linux-gnu-thread-multi

and the result of not using the original patch was:

<h1>Software error:</h1> <pre>Cannot decode string with wide characters at /usr/lib/perl/5.8/Encode.pm line 166. </pre>

I haven't tried the other solutions tested here.
Show 27 quoted lines
>> eval { $res = decode_utf8(...); }
>> if ($@) 
>>      return decode(...);
>> return $res
>>
>> or
>>
>> eval { $res = decode_utf8(...); }
>> if (defined $res)
>>       return $res;
>> else
>>     return decode(...);
>>
>> show the same (wrong) behaviour on Debian sarge. They do not always
>> decode non UTF-8 characters correctly, eg.
>> #öäü does not work
>> #äöüä does work
>>
>> On Debian etch, both versions are working.
>>     
>
> I don't know enough Perl to decide if it is a bug in gitweb usage
> of decode_utf8, if it is a bug in your version of Encode, or if it
> is bug in Encode.
>
> Send copy of this mail to maintainers of Encode perl module.
>   
Ismail do you know if sid was also broken?
Ismail Dönmez· Dec 3, 2007, 22:20 UTC · re: Benjamin Close · lore

Re: Fix UTF Encoding issue

Monday 03 December 2007 Tarihinde 23:46:24 yazmıştı:
Show 30 quoted lines
> Jakub Narebski wrote:
> > On Mon, 3 Dec 2007, Martin Koegler wrote:
> >> On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
> >>> Ismail Dönmez <ismail@pardus.org.tr> writes:
> >>>> Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
> >>>>> Benjamin Close <Benjamin.Close@clearchain.com> writes:
> >>>>>> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> >>>>>> -	if (defined $res) {
> >>>>>> -		return $res;
> >>>>>> -	} else {
> >>>>>> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >>>>>> -	}
> >>>>>> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
> >>>>>> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >>>>>>  }
> >>
> >> This version is broken on Debian sarge and etch. Feeding a UTF-8 and a
> >> latin1 encoding of the same character sequence yields to different
> >> results.
>
> For the record, this was on a debian sid machine.
>
> #perl --version
> This is perl, v5.8.8 built for x86_64-linux-gnu-thread-multi
>
> and the result of not using the original patch was:
>
> <h1>Software error:</h1>
> <pre>Cannot decode string with wide characters at
> /usr/lib/perl/5.8/Encode.pm line 166. </pre>
Can you try the attached patch?
-- 
Never learn by your mistakes, if you do you may never dare to try again.


--- gitweb/gitweb.perl	2007-11-28 11:33:14.000000000 +0200
+++ gitweb/gitweb.perl	2007-11-28 11:33:42.000000000 +0200
@@ -2159,7 +2159,7 @@
 	}
 	my $owner = $gcos;
 	$owner =~ s/[,;].*$//;
-	return to_utf8($owner);
+	return $owner;
 }
 
 ## ......................................................................
Benjamin Close· Dec 3, 2007, 23:04 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

On Tue, Dec 04, 2007 at 12:20:26AM +0200, Ismail D??nmez wrote:
Show 33 quoted lines
> Monday 03 December 2007 Tarihinde 23:46:24 yazm????t??:
> > Jakub Narebski wrote:
> > > On Mon, 3 Dec 2007, Martin Koegler wrote:
> > >> On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
> > >>> Ismail D??nmez <ismail@pardus.org.tr> writes:
> > >>>> Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
> > >>>>> Benjamin Close <Benjamin.Close@clearchain.com> writes:
> > >>>>>> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> > >>>>>> -	if (defined $res) {
> > >>>>>> -		return $res;
> > >>>>>> -	} else {
> > >>>>>> -		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> > >>>>>> -	}
> > >>>>>> +	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); };
> > >>>>>> +	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> > >>>>>>  }
> > >>
> > >> This version is broken on Debian sarge and etch. Feeding a UTF-8 and a
> > >> latin1 encoding of the same character sequence yields to different
> > >> results.
> >
> > For the record, this was on a debian sid machine.
> >
> > #perl --version
> > This is perl, v5.8.8 built for x86_64-linux-gnu-thread-multi
> >
> > and the result of not using the original patch was:
> >
> > <h1>Software error:</h1>
> > <pre>Cannot decode string with wide characters at
> > /usr/lib/perl/5.8/Encode.pm line 166. </pre>
> 
> Can you try the attached patch?
I confirm that the patch corrects the problem.

Without it I get the Cannot decode string error. With it gitweb displays correctly.

Cheers,
	Benjamin
Jakub Narebski· Dec 3, 2007, 23:37 UTC · re: Benjamin Close · lore

Re: Fix UTF Encoding issue

On Tue, 4 Dec 2007, Benjamin Close wrote:
Show 8 quoted lines
> On Tue, Dec 04, 2007 at 12:20:26AM +0200, Ismail Donmez wrote:
> > 
> > Can you try the attached patch?
> 
> I confirm that the patch corrects the problem.
> 
> Without it I get the Cannot decode string error. With it gitweb
> displays correctly.

But the patch _avoids_ issue (des not convert owner to utf8), rather than solving it, if I understand it correctly. What if gecos is in utf-8?

-- 
Jakub Narebski
Poland
Ismail Dönmez· Dec 4, 2007, 04:12 UTC · re: Jakub Narebski · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 Tarihinde 01:37:35 yazmıştı:
Show 12 quoted lines
> On Tue, 4 Dec 2007, Benjamin Close wrote:
> > On Tue, Dec 04, 2007 at 12:20:26AM +0200, Ismail Donmez wrote:
> > > Can you try the attached patch?
> >
> > I confirm that the patch corrects the problem.
> >
> > Without it I get the Cannot decode string error. With it gitweb
> > displays correctly.
>
> But the patch _avoids_ issue (des not convert owner to utf8), rather
> than solving it, if I understand it correctly. What if gecos is in
> utf-8?

Indeed its a workaround but UTF-8 username is correctly displayed in gitweb so my understanding was gecos field is already UTF-8.

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Martin Koegler· Dec 4, 2007, 08:04 UTC · re: Benjamin Close · lore

Re: Fix UTF Encoding issue

On Tue, Dec 04, 2007 at 08:16:24AM +1030, Benjamin Close wrote:
Show 36 quoted lines
> Jakub Narebski wrote:
> >On Mon, 3 Dec 2007, Martin Koegler wrote:
> >>On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
> >>>Ismail Dönmez <ismail@pardus.org.tr> writes:
> >>>>Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
> >>>>>Benjamin Close <Benjamin.Close@clearchain.com> writes:
> >>>>>>-	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> >>>>>>-	if (defined $res) {
> >>>>>>-		return $res;
> >>>>>>-	} else {
> >>>>>>-		return decode($fallback_encoding, $str, 
> >>>>>>Encode::FB_DEFAULT);
> >>>>>>-	}
> >>>>>>+	eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); 
> >>>>>>};
> >>>>>>+	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> >>>>>> }
> >>>>>>            
> >>This version is broken on Debian sarge and etch. Feeding a UTF-8 and a 
> >>latin1
> >>encoding of the same character sequence yields to different results.
>
> For the record, this was on a debian sid machine.
> 
> #perl --version
> This is perl, v5.8.8 built for x86_64-linux-gnu-thread-multi
> 
> and the result of not using the original patch was:
> 
> <h1>Software error:</h1>
> <pre>Cannot decode string with wide characters at 
> /usr/lib/perl/5.8/Encode.pm line 166.
> </pre>
> 
> 
> I haven't tried the other solutions tested here.
Debian etch also has v5.8.8.
My main question is, why is the error not catched?

I'm not a perl programmer, but in your patch the first line is a NOP. The return in eval seems to only returns from the eval block, so any text is decoded as latin1 with the second statement.

In the original version, decode($fallback_encoding, $str, Encode::FB_DEFAULT) can not emit an error, else it would in your version too.

In your version, eval is able to surpress the error of decode_utf8($str, Encode::FB_CROAK);, but not in the original version.

Strange.
mfg Martin Kögler
Ismail Dönmez· Dec 4, 2007, 08:12 UTC · re: Martin Koegler · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 10:04:07 Martin Koegler yazmıştı:
Show 52 quoted lines
> On Tue, Dec 04, 2007 at 08:16:24AM +1030, Benjamin Close wrote:
> > Jakub Narebski wrote:
> > >On Mon, 3 Dec 2007, Martin Koegler wrote:
> > >>On Mon, Dec 03, 2007 at 04:06:48AM -0800, Jakub Narebski wrote:
> > >>>Ismail Dönmez <ismail@pardus.org.tr> writes:
> > >>>>Monday 03 December 2007 Tarihinde 12:14:43 yazm??t?:
> > >>>>>Benjamin Close <Benjamin.Close@clearchain.com> writes:
> > >>>>>>-	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> > >>>>>>-	if (defined $res) {
> > >>>>>>-		return $res;
> > >>>>>>-	} else {
> > >>>>>>-		return decode($fallback_encoding, $str,
> > >>>>>>Encode::FB_DEFAULT);
> > >>>>>>-	}
> > >>>>>>+	eval { return ($res = decode_utf8($str, Encode::FB_CROAK));
> > >>>>>>};
> > >>>>>>+	return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> > >>>>>> }
> > >>
> > >>This version is broken on Debian sarge and etch. Feeding a UTF-8 and a
> > >>latin1
> > >>encoding of the same character sequence yields to different results.
> >
> > For the record, this was on a debian sid machine.
> >
> > #perl --version
> > This is perl, v5.8.8 built for x86_64-linux-gnu-thread-multi
> >
> > and the result of not using the original patch was:
> >
> > <h1>Software error:</h1>
> > <pre>Cannot decode string with wide characters at
> > /usr/lib/perl/5.8/Encode.pm line 166.
> > </pre>
> >
> >
> > I haven't tried the other solutions tested here.
>
> Debian etch also has v5.8.8.
>
> My main question is, why is the error not catched?
>
> I'm not a perl programmer, but in your patch the first line is a
> NOP. The return in eval seems to only returns from the eval block, so
> any text is decoded as latin1 with the second statement.
>
> In the original version, decode($fallback_encoding, $str,
> Encode::FB_DEFAULT) can not emit an error, else it would in your
> version too.
>
> In your version, eval is able to surpress the error of
> decode_utf8($str, Encode::FB_CROAK);, but not in the original version.
I think just a better method is to use (not tested):
if( is_utf8($str) ) 
{
	return decode_utf8($str);
}
else {
	return decode($str);
}

Regards, ismail

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Martin Koegler· Dec 4, 2007, 08:20 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

On Tue, Dec 04, 2007 at 10:12:50AM +0200, Ismail Dönmez wrote:
Show 9 quoted lines
> I think just a better method is to use (not tested):
> 
> if( is_utf8($str) ) 
> {
> 	return decode_utf8($str);
> }
> else {
> 	return decode($str);
> }

I already tried this function. It does not test, if a string is really UTF-8. It seems to be to intended to check, if perl stores the string internally in a multi byte encoding.

mfg Martin Kögler.
Martin Koegler· Dec 4, 2007, 07:50 UTC · re: Jakub Narebski · lore

Re: Fix UTF Encoding issue

On Mon, Dec 03, 2007 at 06:02:54PM +0100, Jakub Narebski wrote:
Show 26 quoted lines
> On Mon, 3 Dec 2007, Martin Koegler wrote:
> > eval { $res = decode_utf8(...); }
> > if ($@) 
> >      return decode(...);
> > return $res
> > 
> > or
> > 
> > eval { $res = decode_utf8(...); }
> > if (defined $res)
> >       return $res;
> > else
> >     return decode(...);
> > 
> > show the same (wrong) behaviour on Debian sarge. They do not always
> > decode non UTF-8 characters correctly, eg.
> > #öäü does not work
> > #äöüä does work
> > 
> > On Debian etch, both versions are working.
> 
> I don't know enough Perl to decide if it is a bug in gitweb usage
> of decode_utf8, if it is a bug in your version of Encode, or if it
> is bug in Encode.
> 
> Send copy of this mail to maintainers of Encode perl module.

The bug affects old versions of perl (Debian sarge = oldstable). As it works on the newer Debian etch, do you really think, that it is a good idea to report issue?

How would you handle a bug report, which reports a bug for gitweb in GIT 1.4, and tells you, that a newer versions works?

As Debian sarge has reached its end of life, the distribution will probable also issue no update.

mfg Martin Kögler
Ismail Dönmez· Dec 4, 2007, 07:55 UTC · re: Martin Koegler · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 Tarihinde 09:50:28 yazmıştı:
> The bug affects old versions of perl (Debian sarge = oldstable).
> As it works on the newer Debian etch, do you really think, that it is
> a good idea to report issue?
Same problem here with v5.8.8 which is latest stable perl5 release.

Regards, ismail

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Martin Koegler· Dec 4, 2007, 08:16 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

On Tue, Dec 04, 2007 at 09:55:04AM +0200, Ismail Dönmez wrote:
Show 6 quoted lines
> Tuesday 04 December 2007 Tarihinde 09:50:28 yazm????t??:
> > The bug affects old versions of perl (Debian sarge = oldstable).
> > As it works on the newer Debian etch, do you really think, that it is
> > a good idea to report issue?
> 
> Same problem here with v5.8.8 which is latest stable perl5 release.

I have put together a small perl script, which tests the various ways of decoding, which have been posted on the list. The first test is wrong by design. A working decoding method should result in "#öäü#äöü".

Debian sarge: #öäü#ÀöÌ ##äöü ##äöü ##äöü

Debian etch, OpenSuSE 10.2, Fedora 7: #öäü#ÀöÌ #öäü#äöü #öäü#äöü #öäü#äöü

mfg Martin Kögler

#!/usr/bin/perl use Encode;

sub t { my $str = shift; my ($res); eval { return ($res = decode_utf8($str, Encode::FB_CROAK)); }; return decode("latin1", $str, Encode::FB_DEFAULT); } sub t1 { my $str = shift; my ($res); eval { ($res = decode_utf8($str, Encode::FB_CROAK)); }; if ($@) { return decode("latin1", $str, Encode::FB_DEFAULT); } else { return $res; } }

sub t2 { my $str = shift; my ($res);

eval { $res = decode_utf8($str, Encode::FB_CROAK); };
 if (defined $res) {
        return $res;
} else {
        return decode("latin1", $str, Encode::FB_DEFAULT);
}
}
sub t3 {
	my $str = shift;
	my $res;
	eval { $res = decode_utf8 ($str, 1); };
	return $res || decode('latin1', $str);
}

print t("#öäü"); print t("#ÀöÌ"); print "\n"; print t1("#öäü"); print t1("#ÀöÌ"); print "\n"; print t2("#öäü"); print t2("#ÀöÌ"); print "\n"; print t3("#öäü"); print t3("#ÀöÌ"); print "\n";

Ismail Dönmez· Dec 4, 2007, 08:28 UTC · re: Martin Koegler · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 10:16:34 Martin Koegler yazmıştı: [...]

> print t("#öäü");
> print t("#ÀöÌ");
> print "\n";

How about this one, doesn't even use Encode, uses just built-in utf8 function :

[~]> cat test.pl binmode STDOUT, ':utf8';

my $str = "#öäü";
if (utf8::valid($str))
{
    utf8::decode($str);
}
print $str."\n";

[~]> perl test.pl #öäü

Regards, ismail

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Ismail Dönmez· Dec 4, 2007, 08:33 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 10:28:59 Ismail Dönmez yazmıştı:
Show 24 quoted lines
> Tuesday 04 December 2007 10:16:34 Martin Koegler yazmıştı:
> [...]
>
> > print t("#öäü");
> > print t("#ÀöÌ");
> > print "\n";
>
> How about this one, doesn't even use Encode, uses just built-in utf8
> function :
>
> [~]> cat test.pl
> binmode STDOUT, ':utf8';
>
> my $str = "#öäü";
>
> if (utf8::valid($str))
> {
>     utf8::decode($str);
> }
>
> print $str."\n";
>
> [~]> perl test.pl
> #öäü
Following to_utf8 function works for me :

sub to_utf8 { ·   my $str = shift;

    if(utf8::valid($str))
    {
        utf8::decode($str);
    }
·
    return $str;
}

Regards, ismail

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Martin Koegler· Dec 4, 2007, 08:44 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

On Tue, Dec 04, 2007 at 10:33:39AM +0200, Ismail Dönmez wrote:
> Following to_utf8 function works for me :
For me too (Debian sarge+etch).
Show 9 quoted lines
> sub to_utf8 {
> ·   my $str = shift;
> 
>     if(utf8::valid($str))
>     {
>         utf8::decode($str);
>     }
> ·
>     return $str;

In the original thread, there was some discussion, that some people might want a different fallback endcoding. So mayme you should keep the second call to decode for the fallback encoding.

> }
mfg Martin Kögler
Ismail Dönmez· Dec 4, 2007, 08:47 UTC · re: Martin Koegler · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 10:44:12 Martin Koegler yazmıştı:
> On Tue, Dec 04, 2007 at 10:33:39AM +0200, Ismail Dönmez wrote:
> > Following to_utf8 function works for me :
>
> For me too (Debian sarge+etch).
Thanks for testing.
Show 13 quoted lines
> > sub to_utf8 {
> > ·   my $str = shift;
> >
> >     if(utf8::valid($str))
> >     {
> >         utf8::decode($str);
> >     }
> > ·
> >     return $str;
>
> In the original thread, there was some discussion, that some people
> might want a different fallback endcoding. So mayme you should
> keep the second call to decode for the fallback encoding.
Probably, I just wanted to fix this damn UTF-8 bug surfacing over and over =)

Regards, ismail

-- 
Never learn by your mistakes, if you do you may never dare to try again.
Ismail Dönmez· Dec 4, 2007, 08:55 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

Tuesday 04 December 2007 10:47:39 Ismail Dönmez yazmıştı:
Show 7 quoted lines
> Tuesday 04 December 2007 10:44:12 Martin Koegler yazmıştı:
> > On Tue, Dec 04, 2007 at 10:33:39AM +0200, Ismail Dönmez wrote:
> > > Following to_utf8 function works for me :
> >
> > For me too (Debian sarge+etch).
>
> Thanks for testing.
Use Perl built-in utf8 function for UTF-8 decoding.
Signed-off-by: İsmail Dönmez <ismail@pardus.org.tr>
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ff5daa7..db255c1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -695,10 +695,9 @@ sub validate_refname {
 # in utf-8 thanks to "binmode STDOUT, ':utf8'" at beginning
 sub to_utf8 {
 	my $str = shift;
-	my $res;
-	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
-	if (defined $res) {
-		return $res;
+        if (utf8::valid($str)) {
+                utf8::decode($str);
+                return $str;
 	} else {
 		return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
 	}
-- 
Never learn by your mistakes, if you do you may never dare to try again.
Jakub Narebski· Dec 4, 2007, 09:07 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

On Tue, 4 Dec 2007, Ismail Dönmez wrote:
> Use Perl built-in utf8 function for UTF-8 decoding.
> 
> Signed-off-by: İsmail Dönmez <ismail@pardus.org.tr>
 
Looks nice. I have not tested it, but if it works: Ack.
-- 
Jakub Narebski
Poland
Wincent Colaiuta· Dec 4, 2007, 10:11 UTC · re: Ismail Dönmez · lore

Re: Fix UTF Encoding issue

El 4/12/2007, a las 9:55, Ismail Dönmez escribió:
Show 28 quoted lines
> Tuesday 04 December 2007 10:47:39 Ismail Dönmez yazmıştı:
>> Tuesday 04 December 2007 10:44:12 Martin Koegler yazmıştı:
>>> On Tue, Dec 04, 2007 at 10:33:39AM +0200, Ismail Dönmez wrote:
>>>> Following to_utf8 function works for me :
>>>
>>> For me too (Debian sarge+etch).
>>
>> Thanks for testing.
>
> Use Perl built-in utf8 function for UTF-8 decoding.
>
> Signed-off-by: İsmail Dönmez <ismail@pardus.org.tr>
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index ff5daa7..db255c1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -695,10 +695,9 @@ sub validate_refname {
> # in utf-8 thanks to "binmode STDOUT, ':utf8'" at beginning
> sub to_utf8 {
> 	my $str = shift;
> -	my $res;
> -	eval { $res = decode_utf8($str, Encode::FB_CROAK); };
> -	if (defined $res) {
> -		return $res;
> +        if (utf8::valid($str)) {
> +                utf8::decode($str);
> +                return $str;

This is good as it fixes another problem which some may have encountered. On at least one distro that I use (Red Hat Enterprise Linux 3) the Encode module is very old (it's 1.83; the latest release is 2.23), and so gitweb won't even run, dying during compilation with this:

	Too many arguments for Encode::decode_utf8 at gitweb.cgi line 686,  
near "Encode::FB_CROAK)"

Of course, the workaround is to install a newer version of the module, but this patch eliminates that dependency which is IMO a good thing.

Cheers, Wincent

← back to recent threads