Closed Bug 360936 Opened 13 years ago Closed 13 years ago

XML parsing regression?

Categories

(Core :: XML, defect)

1.8 Branch
x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: chofmann, Assigned: peterv)

References

Details

(Keywords: fixed1.8.0.9, fixed1.8.1.1)

Attachments

(3 files)

Starting last week we began getting XML parsing errors on minimo start up, due to some strange response to parsing comments in minimo UI files.

Marcio has the test case.
I hope the screenshot helps, it shows that an XML error. The actual error was eliminated when I removed the License Block Comment section.
BTW a small size comment would not cause any problem. As in <!-- foo --> but if you keep adding characters, when the error happens.
Starting when last week?  And did it start last week on _trunk_ as the version field says?  Or on one of the branches?  If so, which one(s)?

In any case, if there's a testcase, can you attach the testcase instead of a screenshot of the testcase?
Will try to figure this out tomorrow with Marcio, apparently Firefox loads the file without a problem.
Version: Trunk → 1.8 Branch
It started with Minimo using Branch 1.8 on Nov 11th. 
REMINDER: 

I checked in a workaround to some XUL minimo source files. the size of the XML license block was reduced and points to a separated license file. 

As this bug gets fixed, then we have to remove that workaround and probably use the standard license block again to each of the source files. 



Flags: blocking1.8.1.1?
Until we understand what's causing this bug and whether or not it can be reproduced on the branch, we can't block on it. If we can reliably trace this to the XML parser, please set the wanted? flag.
Flags: blocking1.8.1.1? → blocking1.8.1.1-
Has anyone confirmed whether or not this bug is a regression from bug 335047?  Chofmann made a note of it there on 11/16.
definitely a regression, but its not clear about how widespread the problem might be.  Its possible that it is limited to WinCE.  Maybe related to some difference in the file system or some other OS quirk on that platform that confuses the new changes in the parser.

On 11/28/06, Doug Turner <dougt@meer.net> wrote:
> I can reproduce the problem.  backing out the changes to the xmlparse.c
> does fix the problem.  I tried backing out of the performance based
> preference but that did not work. 
I am going to try to write some logging code for the branch, so that we can figure out what is going on here and in bug 361180. It is hard to fix these bugs if I can't reproduce them myself.
post a patch and I can try it (ideally, printf's are the way to go... but I will understand if you use NSPR Logging)

Regards,
Doug
It is not limited to WinCE.  I can reproduce on a Linux handset (kernel 2.4.19) running on armv5tel (XScale-Bulverde rev 7).  Perhaps the problem is ARM related?  

pointer alignment?  signed vs unsigned chars?  

(In reply to comment #9)
> definitely a regression, but its not clear about how widespread the problem
> might be.  Its possible that it is limited to WinCE.  Maybe related to some
> difference in the file system or some other OS quirk on that platform that
> confuses the new changes in the parser.
> 
> On 11/28/06, Doug Turner <dougt@meer.net> wrote:
> > I can reproduce the problem.  backing out the changes to the xmlparse.c
> > does fix the problem.  I tried backing out of the performance based
> > preference but that did not work. 
> 

are you running minimo on arm, or ff?
I'm running minimo on arm-linux.  I don't think I'd be able to fit the whole FF on my handset, or I would try it.  

If the problem is with pointer alignment, perhaps it would show up in a Sparc build of FF?


(In reply to comment #13)
> are you running minimo on arm, or ff?
> 

Could someone try this patch and report whether it fixes the bug?
Assignee: xml → peterv
Status: NEW → ASSIGNED
Attached patch LoggingSplinter Review
This patch adds logging code to nsExpatDriver on the branch. If attachment 247068 [details] [diff] [review] doesn't fix the bug, then I'll need the prlog output (NSPR_LOG_MODULES=expatdriver:5).
patch 247068 seams to have fix the problem.  Could you explain this fix?  
Comment on attachment 247068 [details] [diff] [review]
Possible patch v1

On the branch when we resume the parser after blocking it for a stylesheet load we pass in the same buffer again, but starting at the position just after the content that blocked the parser. Expat always keeps the remainder of the buffer in its internal buffer when returning, and so we end up with two copies of that part. This patch makes Expat clear its buffer when blocked. It seems odd that this hasn't caused bigger problems before, it's a good thing that we cleaned all this up on trunk.
Attachment #247068 - Flags: superreview?(bzbarsky)
Attachment #247068 - Flags: review?(bzbarsky)
Comment on attachment 247068 [details] [diff] [review]
Possible patch v1

Oh, right.  r+sr=bzbarsky.
Attachment #247068 - Flags: superreview?(bzbarsky)
Attachment #247068 - Flags: superreview+
Attachment #247068 - Flags: review?(bzbarsky)
Attachment #247068 - Flags: review+
Comment on attachment 247068 [details] [diff] [review]
Possible patch v1

This fixes a regression from a security fix that was landed on the branches (bug 335047). It's not very high-risk (there's always a risk in mucking with these buffers).
Attachment #247068 - Flags: approval1.8.1.1?
Attachment #247068 - Flags: approval1.8.0.9?
Comment on attachment 247068 [details] [diff] [review]
Possible patch v1

Approved for 1.8 and 1.8.0 branches, a=jay for drivers.
Attachment #247068 - Flags: approval1.8.1.1?
Attachment #247068 - Flags: approval1.8.1.1+
Attachment #247068 - Flags: approval1.8.0.9?
Attachment #247068 - Flags: approval1.8.0.9+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
*** Bug 361180 has been marked as a duplicate of this bug. ***
Has this been checked into MOZILLA_1_8_BRANCH ?
(In reply to comment #23)
> Has this been checked into MOZILLA_1_8_BRANCH ?
> 

Sorry for that. I realize now that this is in 1.8.1.1
You need to log in before you can comment on or make changes to this bug.