New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Length-related XHTML parsing errors

Reported by dec...@blizzard.com, Mar 8 2018 
UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.146 Safari/537.36

Steps to reproduce the problem:
1. Visit https://us.battle.net/account/creation/en/ (fairly consistent repro) or https://us.battle.net/forums/en/ (may require several refreshes)
2. You'll see "This page contains the following errors:
error on line 1 at column 1: Extra content at the end of the document Below is a rendering of the page up to the first error." or in rare instances, you may see "error on line <num_that_depends_on_page> at column <some_column>: AttValue: ' expected

What is the expected behavior?

What went wrong?
This smells like an XML byte buffer boundary issue, similar to this old libxml2 issue that chromium hit in 2016: https://bugzilla.gnome.org/show_bug.cgi?id=760183

Did this work before? Yes 64

Does this work in other browsers? Yes

Chrome version: 65.0.3325.146  Channel: stable
OS Version: 10.0
Flash Version: 

Still working on a concise test case, will upload shortly.

Comment 1 by mkwst@chromium.org, Mar 8 2018

Cc: joelhockey@chromium.org
Labels: RegressedIn-65 M-65
Owner: jcivelli@chromium.org
Status: Assigned (was: Unconfirmed)
jcivelli@: Could this be related to any of the libxml changes you've made in the vaguely recent past? Maybe something like https://chromium.googlesource.com/chromium/src/+/c0e3a8e5486d92bab1dbe36563eea6515ffe9d61?

CCing joelhockey@ and vtsyrklevich@ as well for various rolls in the ~65 timeframe.

Comment 2 by dec...@blizzard.com, Mar 8 2018 

I wasn't able to get this test case smaller. I'm able to repro 100% on Chrome 65 through 67 (Canary), and cannot repro under Chrome 64.
xhtml-byte-test.xhtml
96.0 KB View Download

Comment 3 by jcivelli@chromium.org, Mar 8 2018 

Thanks for the test case, I'll bisect to find when this happened.

Comment 4 by dec...@blizzard.com, Mar 8 2018 

Also affecting https://us.diablo3.com/en/ with about 1/10 reproducibility for me.

Comment 5 by dec...@blizzard.com, Mar 8 2018 

This is a simpler test case. Removing the multi-byte char resolves the issue, or pushing the comment node larger or smaller than near the 64k threshold avoids parsing errors. Perhaps a bug related to incorrect use of character length instead of byte length?
xhtml-byte-test2.xhtml
64.0 KB View Download

Comment 6 Deleted

Comment 7 by jcivelli@chromium.org, Mar 8 2018 

This was introduced by the recent roll of libxml:
https://chromium-review.googlesource.com/c/chromium/src/+/897220

I'll look if I can somehow repro in libxml.

Comment 8 by jcivelli@chromium.org, Mar 9 2018

Cc: -joelhockey@chromium.org jcivelli@chromium.org
Owner: joelhockey@chromium.org
joelhockey@ if I revert 
https://git.gnome.org/browse/libxml2/commit/?id=6e6ae5daa6cd9640c9a83c1070896273e9b30d14
locally, content shell will not complain anymore (using repro test case from #2)
Reassigning to you since you wrote that patch.

Comment 9 by joelhockey@chromium.org, Mar 9 2018 

The roughly 32K of content gets encoded into double-byte encoding (a bit over 64K) inside chrome.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/DecodedDataDocumentParser.cpp?l=67&rcl=10288c4e4928303e40e44565286717725d5748a2

Then in libxml, if a multi-byte char has forced you to use icu, there is some code that limits each chunk to 64K.
https://cs.chromium.org/chromium/src/third_party/libxml/src/encoding.c?l=2222&rcl=32423c4c8bd5ae91c942a170abee4047b601b30b

In libxml one possible fix is to revert the flush at:
https://cs.chromium.org/chromium/src/third_party/libxml/src/xmlIO.c?l=3160&rcl=32423c4c8bd5ae91c942a170abee4047b601b30b

Another possible fix is to remove the code that limits reading to 64K.
https://cs.chromium.org/chromium/src/third_party/libxml/src/encoding.c?l=2222&rcl=32423c4c8bd5ae91c942a170abee4047b601b30b

Or, it looks like the code in chromium where you would flush is somewhere around:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/DocumentLoader.cpp?l=734&rcl=10288c4e4928303e40e44565286717725d5748a2
After calling AppendData, you could call Flush.

Comment 10 by joelhockey@chromium.org, Mar 9 2018 

The CL I have done at crosreview.com/956789 adds a call to libxml flush.

But this is causing some other tests to fail such as LayoutTests/svg/hixie/perf/001.xml which shows error: error on line 146 at column 890: AttValue: ' expected

I might try and find someone who is familiar with the XMLDocumentParser code.

Comment 11 by f...@opera.com, Mar 9 2018 

This sounds a lot like  issue 812148  which I debugged a bit a while ago. I think a minimal fix to try would be:

diff --git a/third_party/libxml/src/parser.c b/third_party/libxml/src/parser.c
index 072b1293ea50..57fc297c00de 100644
--- a/third_party/libxml/src/parser.c
+++ b/third_party/libxml/src/parser.c
@@ -11196,6 +11196,7 @@ xmlParseTryOrFinish(xmlParserCtxtPtr ctxt, int terminate) {
                xmlParserInputBufferPush(ctxt->input->buf, 0, "");
                 xmlBufSetInputBaseCur(ctxt->input->buf->buffer, ctxt->input,
                                       base, current);
+                xmlParseGetLasts(ctxt, &lastlt, &lastgt);
            }
            avail = xmlBufUse(ctxt->input->buf->buffer) -
                    (ctxt->input->cur - ctxt->input->base);

(I've been doing a slightly less pessimistic fix, but haven't gotten around to write a test for it yet since it's a bit of a hassle.)

Comment 12 by joelhockey@chromium.org, Mar 11 2018 

Thanks Fredrik, but that change doesn't seem to have helped.

Comment 13 by joelhockey@chromium.org, Mar 12 2018

Cc: joelhockey@chromium.org
Owner: ----
Status: Available (was: Assigned)
Sorry all, but I'm not able to put any more time into looking at this since my team has been moved off chromium.  I expect that the quickest solution is to revert the change in #c8.  This will possibly make fuzzer unhappy again (see  crbug.com/796804 ), so it is hard to know what the best action is.

Comment 14 by schenney@chromium.org, Mar 13 2018

Labels: -Pri-2 ReleaseBlock-Stable FoundIn-66 FoundIn-67 Target-65 FoundIn-65 OS-Android OS-Chrome OS-Linux OS-Mac Pri-1
Status: Untriaged (was: Available)
I would strongly advocate for reverting https://git.gnome.org/browse/libxml2/commit/?id=6e6ae5daa6cd9640c9a83c1070896273e9b30d14

Who has experience doing that, and merging it back?

This is causing us lots of bugs: https://bugs.chromium.org/p/chromium/issues/detail?id=820561,
https://bugs.chromium.org/p/chromium/issues/detail?id=812148,
https://bugs.chromium.org/p/chromium/issues/detail?id=821333

Adding all the appropriate labels for something of this seriousness.

Comment 15 by cmasso@google.com, Mar 14 2018 

schenney@ please can you revert that change so we can test that all related issues are fixed in canary?

Comment 16 by schenney@chromium.org, Mar 16 2018

Cc: -jcivelli@chromium.org vtsyrklevich@chromium.org
Owner: jcivelli@chromium.org
Status: Assigned (was: Untriaged)
I'm not going to try to revert an XML lib change on trunk because I am likely to screw it up. Someone on the XML team needs to do it, or at least someone familiar with updating third party libs.

Comment 17 by schenney@chromium.org, Mar 16 2018 

 Issue 812148  has been merged into this issue.

Comment 18 by schenney@chromium.org, Mar 16 2018

Cc: schenney@chromium.org sandeepkumars@chromium.org
 Issue 820561  has been merged into this issue.

Comment 19 by schenney@chromium.org, Mar 16 2018 

 Issue 822673  has been merged into this issue.

Comment 20 by jcivelli@chromium.org, Mar 16 2018 

I have no experience with updating libxml either.
I think that the patch needs to be reverted in libxml if it's not right. That means:
- getting a repro test case with xmllint
- submit a patch addressing the issue to libxml
- once the patch has landed uprev libxml to that version

Comment 21 by jcivelli@chromium.org, Mar 16 2018

Cc: jcivelli@chromium.org
Owner: ----
Status: Available (was: Assigned)
Maybe someone from the blink team could take a look?

Comment 22 by schenney@chromium.org, Mar 16 2018

Owner: schenney@chromium.org
Status: Assigned (was: Available)
OK, I'll jump in then.
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 16 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4be2115e0abf80619cbf702d0619520d0c4c868d

commit 4be2115e0abf80619cbf702d0619520d0c4c868d
Author: Stephen Chenney <schenney@chromium.org>
Date: Fri Mar 16 18:30:55 2018

Revert "Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c"

This reverts commit c0a946a4dec56ce5906a78f6e0b0c1f9e12c70b6.

Reason for revert: Breaks content all over the web.

Bug:  820163 ,  822673 ,  820561 ,  812148 ,  821333 

Original change's description:
> Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c
>
> This fixes a number of bugs found on clusterfuzz.
>
> Change-Id: Id2fa1d96e55be1e0483c135c20c20b90a068f4c3
> Bug:  790944 
> Bug:  793715 
> Bug:  796804 
> Bug:  799707 
> Reviewed-on: https://chromium-review.googlesource.com/897220
> Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533953}

TBR=dcheng@chromium.org,scottmg@chromium.org,joelhockey@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  790944 ,  793715 ,  796804 ,  799707 
Change-Id: Ic6b934d384229b9cf9092d559b865bbe8f278f38
Reviewed-on: https://chromium-review.googlesource.com/966684
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543766}
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/README.chromium
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/chromium/chromium-issue-628581.patch
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/linux/xml2-config
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/HTMLparser.c
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/configure.ac
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/libxml2.spec
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/parser.c
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/parserInternals.c
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/win32/Makefile.msvc
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/win32/configure.js
[delete] https://crrev.com/431c6dbf0a42d0c31c7dccd6553c6c496f1042a0/third_party/libxml/src/win32/libxml2.rc
[modify] https://crrev.com/4be2115e0abf80619cbf702d0619520d0c4c868d/third_party/libxml/src/xmlIO.c

Comment 24 by schenney@chromium.org, Mar 16 2018

Labels: Merge-Request-66
Do we want to let this bake in Canary, or just merge now?

Given the problems did not turn up any Canary or Dev channel bug reports I don't think we'll get much data from a Canary period.

Comment 25 by schenney@chromium.org, Mar 16 2018

Labels: Merge-Request-65

Comment 26 by schenney@chromium.org, Mar 16 2018

Cc: dcheng@chromium.org
Offline discussion w/ dcheng@ and myself argues for a merge to M-66 and M-65.
Project Member

Comment 27 by sheriffbot@chromium.org, Mar 16 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 28 by gov...@chromium.org, Mar 16 2018

Labels: -Merge-Request-65 -Merge-Review-66 Merge-Approved-66 Merge-Approved-65
Approving merge to M65 branch 3325 & M66 branch 3359 per comments #24, #26 and per offline group chat.
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 16 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f92ca1175c89aec344326778c755ba57ef4d314

commit 0f92ca1175c89aec344326778c755ba57ef4d314
Author: Stephen Chenney <schenney@chromium.org>
Date: Fri Mar 16 18:50:19 2018

Revert "Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c"

M-65 stable merge.

This reverts commit c0a946a4dec56ce5906a78f6e0b0c1f9e12c70b6.

Reason for revert: Breaks content all over the web.

Bug:  820163 ,  822673 ,  820561 ,  812148 ,  821333 

Original change's description:
> Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c
>
> This fixes a number of bugs found on clusterfuzz.
>
> Change-Id: Id2fa1d96e55be1e0483c135c20c20b90a068f4c3
> Bug:  790944 
> Bug:  793715 
> Bug:  796804 
> Bug:  799707 
> Reviewed-on: https://chromium-review.googlesource.com/897220
> Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533953}

TBR=dcheng@chromium.org,scottmg@chromium.org,joelhockey@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  790944 ,  793715 ,  796804 ,  799707 
Change-Id: Ic6b934d384229b9cf9092d559b865bbe8f278f38
Reviewed-on: https://chromium-review.googlesource.com/966684
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543766}(cherry picked from commit 4be2115e0abf80619cbf702d0619520d0c4c868d)
Reviewed-on: https://chromium-review.googlesource.com/967021
Cr-Commit-Position: refs/branch-heads/3325@{#714}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/README.chromium
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/chromium/chromium-issue-628581.patch
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/linux/xml2-config
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/HTMLparser.c
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/configure.ac
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/libxml2.spec
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/parser.c
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/parserInternals.c
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/win32/Makefile.msvc
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/win32/configure.js
[delete] https://crrev.com/483290671a61fdd75600a7b7f5e4a940ba814e9b/third_party/libxml/src/win32/libxml2.rc
[modify] https://crrev.com/0f92ca1175c89aec344326778c755ba57ef4d314/third_party/libxml/src/xmlIO.c
Project Member

Comment 30 by bugdroid1@chromium.org, Mar 16 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54a1c705833b375b124b014159dcadda02a80e9b

commit 54a1c705833b375b124b014159dcadda02a80e9b
Author: Stephen Chenney <schenney@chromium.org>
Date: Fri Mar 16 19:00:42 2018

Revert "Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c"

This reverts commit c0a946a4dec56ce5906a78f6e0b0c1f9e12c70b6.

M-66 merge.

Reason for revert: Breaks content all over the web.

Bug:  820163 ,  822673 ,  820561 ,  812148 ,  821333 

Original change's description:
> Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c
>
> This fixes a number of bugs found on clusterfuzz.
>
> Change-Id: Id2fa1d96e55be1e0483c135c20c20b90a068f4c3
> Bug:  790944 
> Bug:  793715 
> Bug:  796804 
> Bug:  799707 
> Reviewed-on: https://chromium-review.googlesource.com/897220
> Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533953}

TBR=dcheng@chromium.org,scottmg@chromium.org,joelhockey@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  790944 ,  793715 ,  796804 ,  799707 
Change-Id: Ic6b934d384229b9cf9092d559b865bbe8f278f38
Reviewed-on: https://chromium-review.googlesource.com/966684
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543766}(cherry picked from commit 4be2115e0abf80619cbf702d0619520d0c4c868d)
Reviewed-on: https://chromium-review.googlesource.com/966962
Cr-Commit-Position: refs/branch-heads/3359@{#288}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/README.chromium
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/chromium/chromium-issue-628581.patch
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/linux/xml2-config
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/HTMLparser.c
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/configure.ac
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/libxml2.spec
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/parser.c
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/parserInternals.c
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/win32/Makefile.msvc
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/win32/configure.js
[delete] https://crrev.com/11b924f8c4a7c84bfb46e8df78e7ef8d330dc907/third_party/libxml/src/win32/libxml2.rc
[modify] https://crrev.com/54a1c705833b375b124b014159dcadda02a80e9b/third_party/libxml/src/xmlIO.c

Comment 31 by jcivelli@chromium.org, Mar 16 2018 

Reverting the entire libxml roll means reverting other important changes on top of the problematic one.
Is the plan to address the specific problematic patch next?

Comment 32 by schenney@chromium.org, Mar 16 2018

Cc: viswa.karala@chromium.org
 Issue 821536  has been merged into this issue.
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 16 2018

Labels: merge-merged-3372
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8901956103e21b8c3461b779e99cd5d7f50f3ad

commit d8901956103e21b8c3461b779e99cd5d7f50f3ad
Author: Stephen Chenney <schenney@chromium.org>
Date: Fri Mar 16 19:19:12 2018

Revert "Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c"

This reverts commit c0a946a4dec56ce5906a78f6e0b0c1f9e12c70b6.

Canary build branch merge.

Reason for revert: Breaks content all over the web.

Bug:  820163 ,  822673 ,  820561 ,  812148 ,  821333 

Original change's description:
> Roll libxml to 707ad080e61014ab4a6d60dc12875e233c1f673c
>
> This fixes a number of bugs found on clusterfuzz.
>
> Change-Id: Id2fa1d96e55be1e0483c135c20c20b90a068f4c3
> Bug:  790944 
> Bug:  793715 
> Bug:  796804 
> Bug:  799707 
> Reviewed-on: https://chromium-review.googlesource.com/897220
> Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533953}

TBR=dcheng@chromium.org,scottmg@chromium.org,joelhockey@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  790944 ,  793715 ,  796804 ,  799707 
Change-Id: Ic6b934d384229b9cf9092d559b865bbe8f278f38
Reviewed-on: https://chromium-review.googlesource.com/966684
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543766}(cherry picked from commit 4be2115e0abf80619cbf702d0619520d0c4c868d)
Reviewed-on: https://chromium-review.googlesource.com/966690
Cr-Commit-Position: refs/branch-heads/3372@{#1}
Cr-Branched-From: ad7f48548867b059f459e13c53bb8e2e96027381-refs/heads/master@{#543592}
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/README.chromium
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/chromium/chromium-issue-628581.patch
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/linux/xml2-config
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/HTMLparser.c
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/configure.ac
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/libxml2.spec
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/parser.c
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/parserInternals.c
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/win32/Makefile.msvc
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/win32/configure.js
[delete] https://crrev.com/ad7f48548867b059f459e13c53bb8e2e96027381/third_party/libxml/src/win32/libxml2.rc
[modify] https://crrev.com/d8901956103e21b8c3461b779e99cd5d7f50f3ad/third_party/libxml/src/xmlIO.c

Comment 34 by schenney@chromium.org, Mar 16 2018 

Yes to comment #31. I'll start looking at re-enabling each sub-patch from the XML roll to see what we can re-land. Probably won't start until Monday, however. My east coast work day is nearing its end.

Comment 35 by jcivelli@chromium.org, Mar 16 2018 

Thanks schenney@ for looking into this!
If we reland the patches independently in our third-party/libxml, won't we run into problems next time we roll libxml?

Comment 36 by gov...@chromium.org, Mar 16 2018 

Pls verify this fix on canary version 67.0.3372.1+. Thank you.

Comment 37 by wellnho...@aevum.de, Mar 16 2018 

Can anyone confirm that this bug is caused only by the following commit?

    https://git.gnome.org/browse/libxml2/commit/?id=6e6ae5daa6cd9640c9a83c1070896273e9b30d14

If yes, I can revert the commit in libxml2.

Comment 38 by jcivelli@chromium.org, Mar 16 2018 

Yes, this is the commit (https://git.gnome.org/browse/libxml2/commit/?id=6e6ae5daa6cd9640c9a83c1070896273e9b30d14) responsible.
If you could revert it in libxml2 that would be great.

Comment 40 by jcivelli@chromium.org, Mar 16 2018 

Thanks wellnhofer@!
schenney@, should we roll the latest libxml2 instead of reapplying patches manually?

Comment 41 by sandeepkumars@chromium.org, Mar 19 2018

Labels: TE-Verified-M65 TE-Verified-M67 TE-Verified-67.0.3374.0 TE-Verified-65.0.3325.177
Tested the issue using #65.0.3325.177, #67.0.3374.0 on Win 10, Mac 10.13.3 and Linux Debian Rodete as per the steps mentioned in comment #0. No error message is seen.

Please find the screenshot, Hence adding the Verified labels.

Note: 
1. Error is seen in #65.0.3325.146
2. Error isn't seen in #65.0.3325.177.

Thanks!!
Screen Shot 2018-03-19 at 3.57.15 PM.png
75.4 KB View Download

Comment 42 by schenney@chromium.org, Mar 19 2018 

Thanks. I'll look into rolling the latest libxml2 later today.

I need to generate a couple of tests first.

Comment 43 by gov...@chromium.org, Mar 19 2018

Labels: M-67 M-66

Comment 44 by gov...@chromium.org, Mar 19 2018 

 schenney@, could you pls remove "M-65" label from bug if nothing is pending for M65?

Comment 45 by schenney@chromium.org, Mar 19 2018

Labels: -M-65
Yes, moving to M-66 and I'm in the process of converting on of the bug reports into a test case. Then I can make sure the new xmllib works.

Comment 46 by susan.boorgula@chromium.org, Mar 20 2018

Labels: -TE-Verified-65.0.3325.177 TE-Verified-65.0.3325.181
Tested this issue on Windows 10, Mac OS 10.12.6, Ubuntu 14.04 Pro on the reported version 65.0.3325.146 and latest M-65 build 65.0.3325.181.
Able to reproduce this issue on 65.0.3325.146 and the issue is fixed on the latest M-65 build 65.0.3325.181 as per comment #2.

On loading the html file, cannot observe any errors on the page.
Attached is the screen shot for reference.

Hence adding TE verified labels as the fix is working as intended.

Thanks..
820163-M65.PNG
45.8 KB View Download

Comment 47 by ajha@chromium.org, Mar 20 2018

Cc: sindhu.chelamcherla@chromium.org
 Issue 821333  has been merged into this issue.

Comment 48 by gov...@chromium.org, Mar 20 2018 

This patch is now pushing out to stable channel in version 65.0.3325.181 for Desktop (Win,Mac & Linux).

Comment 49 by dec...@blizzard.com, Mar 21 2018 

I can confirm the issue is resolved in 65.0.3325.181. Thank you, everyone.

Comment 50 by susan.boorgula@chromium.org, Mar 21 2018

Labels: TE-Verified-M66 TE-Verified-66.0.3359.45
Able to reproduce this issue on 65.0.3325.146 and the issue is fixed on the latest M-66 build 66.0.3359.45 as per comment #2.

On loading the html file, cannot observe any errors on the page.
Attached is the screen shot for reference.

Hence adding TE verified labels as the fix is working as intended.

Thanks..
820163-M66.PNG
46.6 KB View Download
Project Member

Comment 51 by bugdroid1@chromium.org, Mar 21 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4819d080e4c615633a070572e806721ed5114b0b

commit 4819d080e4c615633a070572e806721ed5114b0b
Author: Stephen Chenney <schenney@chromium.org>
Date: Wed Mar 21 17:17:55 2018

Add a test for xml parsing of long multi-byte encoded files

Verified that this fails with the broken libxml that caused
crbug/820163 and its friends.

R=dcheng@chromium.org
BUG= 820163 , 823230 

Change-Id: I53d631871d4343aaa67c874a90eacbf4d58dc454
Reviewed-on: https://chromium-review.googlesource.com/969408
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544748}
[add] https://crrev.com/4819d080e4c615633a070572e806721ed5114b0b/third_party/WebKit/LayoutTests/xmlviewer/long-multi-byte-char-expected.html
[add] https://crrev.com/4819d080e4c615633a070572e806721ed5114b0b/third_party/WebKit/LayoutTests/xmlviewer/long-multi-byte-char.xml
[add] https://crrev.com/4819d080e4c615633a070572e806721ed5114b0b/third_party/WebKit/LayoutTests/xmlviewer/resources/long-multi-byte-char.xslt

Comment 52 by cmasso@google.com, Mar 23 2018 

Is this now completely fixed in M66?
Project Member

Comment 53 by bugdroid1@chromium.org, Mar 23 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380

commit f01ade19f2ee3e7037bb57acb46e3bd6d45a0380
Author: Stephen Chenney <schenney@chromium.org>
Date: Fri Mar 23 15:36:43 2018

Roll libxml to 7a1bd7f6497ac33a9023d556f6f47a48f01deac0

R=dcheng@chromium.org

Bug:  790944 , 820163 , 793715 , 796804 , 799707 , 823345 
Change-Id: I6daa6aedd8ccff792b99c228d85800dbd2dd3ec2
Reviewed-on: https://chromium-review.googlesource.com/973467
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545458}
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/README.chromium
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/chromium/chromium-issue-628581.patch
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/chromium/roll.py
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/linux/config.h
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/linux/include/libxml/xmlversion.h
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/linux/xml2-config
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/mac/config.h
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/mac/include/libxml/xmlversion.h
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/aclocal.m4
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/configure.ac
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/libxml2.spec
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/libxml2.syms
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/parser.c
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/testapi.c
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/win32/Makefile.msvc
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/win32/configure.js
[add] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/win32/libxml2.rc
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/src/xpath.c
[modify] https://crrev.com/f01ade19f2ee3e7037bb57acb46e3bd6d45a0380/third_party/libxml/win32/include/libxml/xmlversion.h

Comment 54 by schenney@chromium.org, Mar 23 2018 

The bug is currently fixed on M-66. According to the release information I have, we've rolled out the new beta since the fix in comment #30 so Beta users will see it. We;'ve also rolled out new stable so everyone who updates should have the fix now.

We should merge the most recent trunk libxml roll back to M-66 due to the related security issues. But first it needs to bake on Canary for the weekend and probably Monday and Tuesday.

Comment 55 by schenney@chromium.org, Mar 23 2018

Status: Fixed (was: Assigned)
Project Member

Comment 56 by bugdroid1@chromium.org, Mar 27 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e0b7754a48adf46c2d9a352085754d46a404ba1c

commit e0b7754a48adf46c2d9a352085754d46a404ba1c
Author: Stephen Chenney <schenney@chromium.org>
Date: Tue Mar 27 18:06:12 2018

Roll libxml to 7a1bd7f6497ac33a9023d556f6f47a48f01deac0

M-66 Cherry-pick

TBR=​dcheng@chromium.org

Bug:  790944 , 820163 , 793715 , 796804 , 799707 , 823345 
Change-Id: I6daa6aedd8ccff792b99c228d85800dbd2dd3ec2
Reviewed-on: https://chromium-review.googlesource.com/973467
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#545458}(cherry picked from commit f01ade19f2ee3e7037bb57acb46e3bd6d45a0380)
Reviewed-on: https://chromium-review.googlesource.com/981755
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#464}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/README.chromium
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/chromium/chromium-issue-628581.patch
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/chromium/roll.py
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/linux/config.h
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/linux/include/libxml/xmlversion.h
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/linux/xml2-config
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/mac/config.h
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/mac/include/libxml/xmlversion.h
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/aclocal.m4
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/configure.ac
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/libxml2.spec
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/libxml2.syms
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/parser.c
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/testapi.c
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/win32/Makefile.msvc
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/win32/configure.js
[add] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/win32/libxml2.rc
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/src/xpath.c
[modify] https://crrev.com/e0b7754a48adf46c2d9a352085754d46a404ba1c/third_party/libxml/win32/include/libxml/xmlversion.h

Comment 57 by prashanthpola@chromium.org, Mar 28 2018 

Verified on Android Chrome:66.0.3359.67 Device:LG G4 (LG-H815)/MRA58K
►

Sign in to add a comment