Closed Bug 449219 Opened 11 years ago Closed 11 years ago

[FIX]JavaScript code parsing a large XML document returns truncated values

Categories

(Core :: XML, defect, P3, critical)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: bugzilla, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(4 keywords)

Attachments

(5 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

I have an Ajax application which parses XML data extracted from a database by PHP scripts on a remote server. This worked fine with Firefox 2, but with Firefox 3 I started getting random errors. After investigation I found that the code that was parsing the XML was on occasion returning truncated values. This was quite easy to spot since the database contains encrypted data (128 bit AES) represented as hex strings, so all string lengths are exact multiples of 32. I have produced a simple example which demonstrates this; I will attach this to the report.

The example parses an XML file which has been loaded into an IFRAME. If any of the values has a length that is not a multiple of 32 then the details are displayed above the IFRAME, and a count of the number of truncated values detected.

Reproducible: Always

Steps to Reproduce:
1.Unzip attachment.
2.Open the file XMLParseError2.htm in Firefox
3.
Actual Results:  
1774: 8F
Error Count: 1

{XML Data}

Expected Results:  
Error Count: 0

{XML Data}
I assumed in bug report I would be able to upload a zip file containing the test HTML file and associated XML data, but have now uploaded them as separate attachments.
I can confirmed I get the 1774: 8F error using today's build (2008/08/05).

I am using Windows XP SP2
My browser is:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008080403 Minefield/3.1a2pre

This will probably be a "Core" issue or it could be an external component.
Works: 2007013011
Fails: 2007013013

Regression window: 
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2007-01-30+10%3A00&maxdate=2007-01-30+14%3A00
-> bug 18333.
Status: UNCONFIRMED → NEW
Component: General → XML
Ever confirmed: true
Keywords: regression, testcase
Product: Firefox → Core
QA Contact: general → xml
Version: unspecified → Trunk
Flags: blocking1.9.1?
> var id = nextRecord.getElementsByTagName ("id") [0].firstChild.data;
> var value = nextRecord.getElementsByTagName ("value") [0].firstChild.data;

Using .textContent instead would work.  In this case, the value has been split across two textnodes.  The split happens at exactly the 65537th character in the file, which I have a hard time believing is a coincindence.
So yeah, when loading from local disk it looks like we hit a "packet boundary" at that point, call WillInterruptParse(), and nsContentSink::WillInterruptImpl calls FlushTags.  Then the XML content sink appends the text and starts a new textnode.  It really shouldn't do that; instead it should do something closer to what SinkContext::FlushText does.
Attached patch This should fix it (obsolete) — Splinter Review
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #337371 - Attachment is obsolete: true
Attached patch Same as diff -wSplinter Review
Attachment #337373 - Flags: superreview?(jonas)
Attachment #337373 - Flags: review?(jonas)
Summary: JavaScript code parsing a large XML document returns truncated values → [FIX]JavaScript code parsing a large XML document returns truncated values
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P3
Target Milestone: --- → mozilla1.9.1
Blocks: 442094
Comment on attachment 337373 [details] [diff] [review]
Same as diff -w

>+nsXMLContentSink::FlushText(PRBool aReleaseTextNode)
> {
>-  if (mTextLength == 0) {
>-    return NS_OK;
>+  nsresult rv = NS_OK;
>+
>+  if (mTextLength != 0) {
>+    if (mLastTextNode) {
>+      if ((mLastTextNodeSize + mTextLength) > mTextSize) {

It would be great of you added   "&& !mXSLTProcessor"

as well. That would also fix bug 449219. We used to have such a check but it was lost in the incremental xml landing.

r/sr=me either way.
Attachment #337373 - Flags: superreview?(jonas)
Attachment #337373 - Flags: superreview+
Attachment #337373 - Flags: review?(jonas)
Attachment #337373 - Flags: review+
Duplicate of this bug: 459826
Pushed changeset 774533c7cf84.

Not sure how to test this, offhand.  No packet boundary control in mochitest...  :(  Jonas, can you write an XSLT test (presumably using the 4k
Flags: in-testsuite?
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Duplicate of this bug: 459122
Keywords: fixed1.9.1
Target Milestone: mozilla1.9.1 → mozilla1.9.1b2
Boris, wanna port this to the 1.9.0 branch?
Attached patch 1.9.0 branch fixSplinter Review
Sure.  The trunk patch just applies there, with -F4.  But here's a diff after applying, so it just applies cleanly.
Attachment #353925 - Flags: approval1.9.0.6?
Comment on attachment 353925 [details] [diff] [review]
1.9.0 branch fix

Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #353925 - Flags: approval1.9.0.6? → approval1.9.0.6+
Checked in on branch.
Keywords: fixed1.9.0.6
Verified fixed for 1.9.0.6 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2009011304 GranParadiso/3.0.6pre.
You need to log in before you can comment on or make changes to this bug.