Last Comment Bug 423439 - show_bug.cgi crashes when wrapping comments which contain Unicode characters and tabs in them
: show_bug.cgi crashes when wrapping comments which contain Unicode characters ...
Status: RESOLVED FIXED
: regression
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 3.1.3
: All All
: -- major (vote)
: Bugzilla 3.2
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-17 06:59 PDT by Sergey Spatar
Modified: 2010-02-28 12:45 PST (History)
3 users (show)
mkanat: approval+
mkanat: blocking3.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Workaround: trim white-space characters at the end of line before calling wrap(). (416 bytes, patch)
2008-03-17 07:04 PDT, Sergey Spatar
no flags Details | Diff | Review
Test case for wrap() on utf8 string (186 bytes, application/octet-stream)
2008-03-25 01:46 PDT, Sergey Spatar
no flags Details
patch, v1 (820 bytes, patch)
2008-03-27 20:03 PDT, Frédéric Buclin
mkanat: review+
Details | Diff | Review

Description Sergey Spatar 2008-03-17 06:59:00 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: 

Page show_bug.cgi crashes on wrapping comment containing Cyrillic characters and tabulation character at end.
This bug is related to Bug 388723, but the fix provided in Bug 388723 doesn't work in my case.

Reproducible: Always

Steps to Reproduce:
1. Install Bugzilla 3.1.3, enable utf8.
2. Create bug with additional comment string, containing several Latin characters, then space, then several Cyrillic characters, and tabulation character (\t) at the end. For example (use without quotes):
"ABC АБВ	"

Actual Results:  
The show_bug.cgi crashes on Bugzilla/Util.pm in function wrap_comment on line 315:
$wrappedcomment .= (wrap('', '', $line) . "\n");

Expected Results:  
The show_bug.cgi shows created bug correctly.

You shouldn't use function wrap() from Perl module Text::Wrap here, because it doesn't support Unicode.
Comment 1 Sergey Spatar 2008-03-17 07:04:12 PDT
Created attachment 309963 [details] [diff] [review]
Workaround: trim white-space characters at the end of line before calling wrap().
Comment 2 Frédéric Buclin 2008-03-18 05:00:54 PDT
I can reproduce the bug using the following string (without quotes):
"ABC АБВ		hABC АБВ		"

I can no longer view the bug. The page remains blank.
Comment 3 Max Kanat-Alexander 2008-03-22 01:11:31 PDT
This sounds like a bug in Text::Wrap which somebody should investigate. What's the actual error being thrown in the error log when this happens?
Comment 4 Frédéric Buclin 2008-03-24 11:42:18 PDT
(In reply to comment #3)
> What's the actual error being thrown in the error log when this happens?

That's the problem: no error is thrown, nowhere. The page remains blank, nothing else.
Comment 5 Max Kanat-Alexander 2008-03-24 17:34:26 PDT
(In reply to comment #4)
> That's the problem: no error is thrown, nowhere. The page remains blank,
> nothing else.

  Wow. Is it a browser issue? Does it happen in other browsers? 

  Is Perl segfaulting, maybe?
Comment 6 Sergey Spatar 2008-03-25 01:46:48 PDT
Created attachment 311526 [details]
Test case for wrap() on utf8 string

Segfault in Perl:

> gdb perl
GNU gdb 6.4
Copyright 2005 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "x86_64-suse-linux"...(no debugging symbols found)
Using host libthread_db library "/lib64/libthread_db.so.1".

(gdb) run test_wrap_utf8.pl
Starting program: /usr/bin/perl test_wrap_utf8.pl
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
(no debugging symbols found)
[Thread debugging using libthread_db enabled]
[New Thread 47078923111280 (LWP 19469)]
(no debugging symbols found)
This script tests wrap() function.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 47078923111280 (LWP 19469)]
0x00000000004999e7 in Perl_sv_pos_b2u ()
(gdb) bt
#0  0x00000000004999e7 in Perl_sv_pos_b2u ()
#1  0x000000000049f6a5 in Perl_pp_pos ()
#2  0x000000000046515e in Perl_runops_debug ()
#3  0x0000000000424c1c in perl_run ()
#4  0x000000000041d6c4 in main ()
Comment 7 timeless 2008-03-25 02:51:25 PDT
that will need to be reported upstream and we'll need to patch checksetup to ban the bad versions.
Comment 8 Frédéric Buclin 2008-03-25 17:13:15 PDT
Reported upstream:

http://rt.perl.org/rt3/Public/Bug/Display.html?id=52104
Comment 9 Frédéric Buclin 2008-03-26 18:04:07 PDT
As reported upstream at http://rt.perl.org/rt3/Public/Bug/Display.html?id=52104#txn-377006, this bug is due to a problem with pos() and should be fixed in Perl 5.8.9. I don't see what else we can do for Bugzilla. All we can hope is that this fix will be backported.
Comment 10 Max Kanat-Alexander 2008-03-26 18:16:35 PDT
(In reply to comment #9)
> All we can hope is that this fix will be backported.

  Fixing it in 5.8.9 *is* a backport.
Comment 11 Frédéric Buclin 2008-03-27 14:35:52 PDT
(In reply to comment #10)
>   Fixing it in 5.8.9 *is* a backport.

Many Linux distros backport critical fixes in their packages, e.g. Mandriva could backport it in Perl 5.8.8, which is the version of Perl currently supported.
Comment 12 Frédéric Buclin 2008-03-27 20:03:27 PDT
Created attachment 312181 [details] [diff] [review]
patch, v1

OK, per http://rt.perl.org/rt3/Public/Bug/Display.html?id=52104#txn-377490, the problem comes from Text::Tabs::expand() which calls pos() if the string contains a tab. pos() generates a segfault with Unicode and so must be removed from comments to avoid triggering it.

Tabs are seldom used to format a comment. Most of the time, whitespaces are used. So this shouldn't affect many people, especially when you know that you cannot easily insert tabs into comments (you have to copy the text from a text editor to do so). If you really want tabs to be inserted, you could start lines with ">", so that wrap() isn't called.

Putting wrap() into eval{} doesn't seem to work as it's a critical crash (segfault), so we cannot use this workaround, unfortunately.

With this patch applied, I can now again access show_bug.cgi with comments which contain both tabs and Unicode. I don't see a better workaround.
Comment 13 Max Kanat-Alexander 2008-03-27 23:29:57 PDT
Comment on attachment 312181 [details] [diff] [review]
patch, v1

I agree, tabs will rarely be in comments.

We should probably replace them with eight spaces instead of just one, though. I think that's the most common tab-stop size.
Comment 14 timeless 2008-03-28 01:13:23 PDT
shouldn't this be version checked against 5.8.9/5.9.x/5.10?

note that some of us actively use tabs in bugs. anytime we paste tabular output from a webtool:
talkback
sorocco
bonsai

yes, 8 spaces would be better than 1.
there's even a gecko parsing bug involving the various layouts of tabs/spaces.

<span class="tab">        </span>

.span:content("\t")

could be better i suppose.
Comment 15 Frédéric Buclin 2008-03-28 03:20:03 PDT
(In reply to comment #13)
> We should probably replace them with eight spaces instead of just one, though.
> I think that's the most common tab-stop size.

One tab is replaced by 8 whitespaces *at most*. Statistically, it's on average much less, and to be exact: 4. :) Replacing each tab by 8 whitespaces may generate too long lines and make the comment even more unreadable. So we should take 1, 2 or 4 only, IMO.
Comment 16 Frédéric Buclin 2008-03-28 03:22:58 PDT
(In reply to comment #14)
> shouldn't this be version checked against 5.8.9/5.9.x/5.10?

No. For now, *none* of the release has the fix yet, and we have no guarantee that 5.8.9 will have it. We also don't know which release of 5.10 will have it. About 5.9, it clearly don't have the fix now that 5.10 is released.
Comment 17 Frédéric Buclin 2008-03-28 05:34:29 PDT
I finally replaced each tab by four whitespaces.

Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.68; previous revision: 1.67
done

We should relnote that tabs will no longer be rendered correctly in comments, due to this issue.
Comment 18 Max Kanat-Alexander 2008-07-01 00:08:01 PDT
Added to the release notes for Bugzilla 3.2 in a patch on bug 432331.

Note You need to log in before you can comment on or make changes to this bug.


Privacy Policy