Closed Bug 423439 Opened 16 years ago Closed 16 years ago

show_bug.cgi crashes when wrapping comments which contain Unicode characters and tabs in them

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

3.1.3
defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: spatar, Assigned: LpSolit)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

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.
I can reproduce the bug using the following string (without quotes):
"ABC АБВ		hABC АБВ		"

I can no longer view the bug. The page remains blank.
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking3.2?
Keywords: regression
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 3.2
Version: unspecified → 3.1.3
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?
Severity: critical → major
Flags: blocking3.2? → blocking3.2+
(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.
(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?
Attached file Test case for wrap() on utf8 string (obsolete) —
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 ()
that will need to be reported upstream and we'll need to patch checksetup to ban the bad versions.
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.
(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.
(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.
Attached patch patch, v1Splinter Review
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.
Assignee: create-and-change → LpSolit
Attachment #309963 - Attachment is obsolete: true
Attachment #311526 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #312181 - Flags: review?(mkanat)
Summary: Page show_bug.cgi crashes on wrapping comment containing Cyrillic characters → show_bug.cgi crashes when wrapping comments which contain Unicode characters and tabs in them
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.
Attachment #312181 - Flags: review?(mkanat) → review+
Flags: approval+
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.
(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.
(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.
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.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: relnote
Resolution: --- → FIXED
Added to the release notes for Bugzilla 3.2 in a patch on bug 432331.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.