Bug #73177 for Cairo: ->show_text does not handle latin1 strings correctly

Sun Dec 11 12:40:59 2011 SREZIC [...] cpan.org - Ticket created

Subject:

->show_text does not handle latin1 strings correctly

The ->show_text method does not handle codepoints > 128 in a perl string without utf8 flag correctly. See the attached sample script. A quick glance in Cairo.xs shows that SvPV is used. Probably SvPVutf8 should be used instead here to fix the problem. See also "perldoc perlapi". Regards, Slaven

Subject:

cairo_non_utf8.pl

#!/usr/bin/perl use strict; use Cairo; my $surface = Cairo::ImageSurface->create("rgb24", 300, 50); my $context = Cairo::Context->create($surface); my $text = "Test äöüß"; #utf8::upgrade($text); $context->move_to(10,10); $context->set_source_rgb(1,1,1); $context->show_text($text); $surface->write_to_png ('/tmp/test.png'); __END__

Sun Dec 11 15:07:14 2011 kaffeetisch [...] gmx.de - Correspondence added

Subject:	Re: [rt.cpan.org #73177] ->show_text does not handle latin1 strings correctly
Date:	Sun, 11 Dec 2011 21:07:02 +0100
To:	bug-Cairo [...] rt.cpan.org, gtk-perl-list [...] gnome.org
From:	Torsten Schoenfeld <kaffeetisch [...] gmx.de>

On 11.12.2011 18:40, Slaven_Rezic via RT wrote: Show quoted text

> Sun Dec 11 12:40:59 2011: Request 73177 was acted upon. > Transaction: Ticket created by SREZIC > Queue: Cairo > Subject: ->show_text does not handle latin1 strings correctly > Broken in: 1.081 > Severity: (no value) > Owner: Nobody > Requestors: SREZIC@cpan.org > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=73177 > > > > The ->show_text method does not handle codepoints > 128 in a perl string > without utf8 flag correctly. See the attached sample script. > > A quick glance in Cairo.xs shows that SvPV is used. Probably SvPVutf8 > should be used instead here to fix the problem. See also "perldoc perlapi".

Yeah, the current implementation basically burdens the user with ensuring correct encoding. As you show in the attached program, that basically boils down to utf8::upgrade() if you have latin1-encoded strings. I agree that it would be convenient if Cairo auto-upgraded strings if utf8 is expected. But I worry about backwards-compatibility: * If someone used utf8::encode() instead of utf8::upgrade(), then auto-upgrading will lead to double encoding. * If someone used utf8 literals without specifying "use utf8", then auto-upgrading will also lead to double encoding. Technically, both these usages are incorrect. But they work OK with current Cairo and it would break if implemented auto-upgrading. What do you think? I pushed a branch to the git repo which implements the necessary changes: <http://git.gnome.org/browse/perl-Cairo/commit/?h=auto-utf8&id=f40ee313502510aedcd2df1a225fa6095717c2d2>. I also CC the mailing list for further opinions.

Sun Dec 11 15:07:15 2011 The RT System itself - Status changed from 'new' to 'open'

Sun Dec 11 16:36:45 2011 slaven [...] rezic.de - Correspondence added

CC:	SREZIC [...] cpan.org
Subject:	Re: [rt.cpan.org #73177] ->show_text does not handle latin1 strings correctly
Date:	Sun, 11 Dec 2011 22:33:39 +0100
To:	bug-Cairo [...] rt.cpan.org
From:	Slaven Rezic <slaven [...] rezic.de>

"Torsten Schoenfeld via RT" <bug-Cairo@rt.cpan.org> writes: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=73177 > > > On 11.12.2011 18:40, Slaven_Rezic via RT wrote:

>> Sun Dec 11 12:40:59 2011: Request 73177 was acted upon. >> Transaction: Ticket created by SREZIC >> Queue: Cairo >> Subject: ->show_text does not handle latin1 strings correctly >> Broken in: 1.081 >> Severity: (no value) >> Owner: Nobody >> Requestors: SREZIC@cpan.org >> Status: new >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=73177 > >> >> >> The ->show_text method does not handle codepoints > 128 in a perl string >> without utf8 flag correctly. See the attached sample script. >> >> A quick glance in Cairo.xs shows that SvPV is used. Probably SvPVutf8 >> should be used instead here to fix the problem. See also "perldoc perlapi".

> > Yeah, the current implementation basically burdens the user with > ensuring correct encoding. As you show in the attached program, that > basically boils down to utf8::upgrade() if you have latin1-encoded > strings. I agree that it would be convenient if Cairo auto-upgraded > strings if utf8 is expected. But I worry about backwards-compatibility: > > * If someone used utf8::encode() instead of utf8::upgrade(), then > auto-upgrading will lead to double encoding. > > * If someone used utf8 literals without specifying "use utf8", then > auto-upgrading will also lead to double encoding. > > Technically, both these usages are incorrect. But they work OK with > current Cairo and it would break if implemented auto-upgrading. > > What do you think? I pushed a branch to the git repo which implements > the necessary changes: > <http://git.gnome.org/browse/perl-Cairo/commit/?h=auto-utf8&id=f40ee313502510aedcd2df1a225fa6095717c2d2>. > I also CC the mailing list for further opinions. >

The change looks good to me (SvPVutf8 is using sv_utf8_upgrade behind the scenes, so it's more-or-less the same). About backward-compatibility: yes, things may break. But then, Cairo's cousin Pango is doing utf8-stuff already correct. So users are surprised anyway if they pass the same string to Cairo and Pango and get different results. Regards, Slaven -- Slaven Rezic - slaven <at> rezic <dot> de tktimex - time recording tool http://sourceforge.net/projects/ptktools/

Sat Apr 21 12:53:16 2012 kaffeetisch [...] gmx.de - Correspondence added

The change has now been merged into git master. Thanks for the report.

Sat Apr 21 12:53:17 2012 kaffeetisch [...] gmx.de - Status changed from 'open' to 'resolved'