RESOLVED FIXED 71099
Reset line numbers for scripts generated with document.write. 
https://bugs.webkit.org/show_bug.cgi?id=71099
Summary Reset line numbers for scripts generated with document.write.
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
DetailsFormatted DiffDiff
[Patch] With inline handler test case. (8.92 KB, patch)
2011-10-28 06:04 PDT, Pavel Feldman 		yurys: review+
DetailsFormatted DiffDiff
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.