Bug 71099

Summary:

Reset line numbers for scripts generated with document.write.

Product:

WebKit

Reporter:

Pavel Feldman <pfeldman>

Component:

WebCore Misc.

Assignee:

Pavel Feldman <pfeldman>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, podivilov, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
[Patch] With inline handler test case.	yurys: review+

Pavel Feldman

Reported 2011-10-28 02:12:47 PDT

We need to distinguish scripts generated with document.write from the ones generated by the parser of the original resource. Otherwise we bind them to the resource and hence lose them from the debugging UI. Consider the following example: 1: <html> 2: <script>function aa() {}</script> 3: <script>function bb() {}</script> 4: <script>document.write("<scrip" + "t>\n\n\n\n\nfunction cc() {}</sc" + "ript>");</script> 5: <script>function dd() {}</script> 6: </html> As a result, following scripts are reported to be parsed (all sharing the main resource url): line 2-2 line 3-3 line 4-10 line 5-5 As a result of this patch, scripts will be annotated: line 2-2 line 3-3 line 0-6 line 5-5 We will then render scripts starting at 0 as standalone entities.

Attachments
Patch (8.86 KB, patch) 2011-10-28 05:31 PDT, Pavel Feldman	no flags	Details Formatted Diff Diff
[Patch] With inline handler test case. (8.92 KB, patch) 2011-10-28 06:04 PDT, Pavel Feldman	yurys: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Pavel Feldman

Comment 1 2011-10-28 05:31:29 PDT

Created attachment 112851 [details] Patch

Yury Semikhatsky

Comment 2 2011-10-28 05:46:49 PDT

Comment on attachment 112851 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112851&action=review > LayoutTests/http/tests/inspector-enabled/document-write.html:5 > +<script>console.log("Line 5");</script> Please test also that JS handlers written in html attributes will have positions different from scripts created with document.write.

Pavel Feldman

Comment 3 2011-10-28 06:04:04 PDT

Created attachment 112852 [details] [Patch] With inline handler test case.

Yury Semikhatsky

Comment 4 2011-10-28 06:17:22 PDT

Comment on attachment 112852 [details] [Patch] With inline handler test case. View in context: https://bugs.webkit.org/attachment.cgi?id=112852&action=review > LayoutTests/http/tests/inspector-enabled/document-write.html:27 > +Tests that console reports zero line number for scripts generated with document.write. Please add a link to the bug in this description.

Pavel Feldman

Comment 5 2011-10-28 06:27:53 PDT

Committed r98724: <http://trac.webkit.org/changeset/98724>

Adam Barth

Comment 6 2011-10-28 11:15:05 PDT

Comment on attachment 112852 [details] [Patch] With inline handler test case. View in context: https://bugs.webkit.org/attachment.cgi?id=112852&action=review > Source/WebCore/dom/ScriptElement.cpp:201 > + Document* document = m_element->document(); In general it's dangerous to store references like this in a raw pointer. If anything in this script executes script between this program point and any use of this variable, you're going to have a security vulnerability. It's much better to continue to access the document via some protected point.

Pavel Feldman

Comment 7 2011-10-28 11:28:34 PDT

> In general it's dangerous to store references like this in a raw pointer. If anything in this script executes script between this program point and any use of this variable, you're going to have a security vulnerability. It's much better to continue to access the document via some protected point. I went through the code originally and have not noticed any uncontrolled lack of control flow / re-enterability. I can roll this refactoring back though in order to maintain the security pattern you suggest in case it is common to WebCore.

Adam Barth

Comment 8 2011-10-28 11:30:00 PDT

I checked them too and I didn't see any problem. It's just a trap waiting to catch someone. It's up to you whether you'd like to change it back.

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