Bug 26555 – Fix Chromium canary bot

• Component: DOM
• Status: RESOLVED FIXED

[Adam Barth]

The Chromium canary bot is unhappy with some parts of the XSSAuditor.  Patch forthcoming.

[Adam Barth]

Created attachment 31559 [details]
patch

[Dimitri Glazkov (Google)]

Comment on attachment 31559 [details]
patch

yay!

[Adam Barth]

Will land.  DRT is chugging along as we speak.

[Adam Barth]

Sending        WebCore/ChangeLog
Sending        WebCore/bindings/js/ScriptController.cpp
Sending        WebCore/bindings/js/ScriptSourceCode.h
Sending        WebCore/bindings/v8/ScriptController.cpp
Sending        WebCore/page/XSSAuditor.cpp
Sending        WebCore/page/XSSAuditor.h
Transmitting file data ......
Committed revision 44869.

[Darin Adler]

The source() function in ScriptSourceCode.h looks bad to me. It looks like JavaScriptCore is being forced to copy some code -- won't that make things slow?

[David Levin]

I agree with Darin.

It looks like that copy was in there before *but* it was only done when m_isEnabled was true.

59  bool XSSAuditor::canEvaluate(const ScriptSourceCode& sourceCode) const
60  {
61      if (!m_isEnabled)
62          return true;
63      
64      return canEvaluate(String(sourceCode.jsSourceCode().data(), sourceCode.jsSourceCode().length()));

A simple fix to restore old behavior would be to change this line
   84     if (!m_XSSAuditor->canEvaluate(sourceCode.source())) {
to 
 84     if (m_XSSAuditor->isEnabled() && !m_XSSAuditor->canEvaluate(sourceCode.source())) {

[Adam Barth]

(In reply to comment #5)
> The source() function in ScriptSourceCode.h looks bad to me. It looks like
> JavaScriptCore is being forced to copy some code -- won't that make things
> slow?

Maybe ScriptSourceCode should just grab a reference to the string on construction?  It looks like the string is kept alive anyway because the ScriptSourceCode holds a JSC::SourceCode which holds a RefPtr<SourceProvider> which holds String m_source (via StringSourceProvider : public JSC::SourceProvider).

[Adam Barth]

Created attachment 31572 [details]
work-in-progress patch

[Adam Barth]

Follow patch in https://bugs.webkit.org/show_bug.cgi?id=26561