[Webkit-unassigned] [Bug 26199] Implement a reflective XSS filter

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26199


abarth at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31109|0                           |1
        is obsolete|                            |
  Attachment #31154|review?                     |
               Flag|                            |
  Attachment #31154|0                           |1
        is obsolete|                            |
  Attachment #31269|                            |review?
               Flag|                            |




------- Comment #16 from abarth at webkit.org  2009-06-14 13:50 PDT -------
Created an attachment (id=31269)
 --> (https://bugs.webkit.org/attachment.cgi?id=31269&action=view)
updated patch

(In reply to comment #15)
> Why activeXSSAuditor?  Will there be more than one per scriptController in the
> future?

Fixed.

> > +    // The XSSAuditor associated with this ScriptController.
> > +    OwnPtr<XSSAuditor> m_XSSAuditor;
> 
> Does the auditor have to be a pointer here? 

I make this a pointer because I didn't want to include XSSAuditor.h here
because ScriptController.h is included by the entire world (including outside
of WebCore).

> We don't use @param syntax.  In fact, neither of these comments are really
> necessary as the code is self explanatory.

Fixed.

> The message should be a static.

Fixed x3.

> > +// static
> 
> This comment is not necessary.

Fixed.

> Missing space after (allowNullCharacters).  The parens are not needed as well.

Fixed.

> The use of # is inconsistent with the rest of the comment.

Fixed.

> We don't use @param syntax.  In fact, this comment is not necessary.

Fixed.

> Overall this looks great, but I want to defer r+ing this until we have some
> performance analysis done on it, even it only in the disabled state.

You mean you'd like to measure the performance impact when its enabled, right?

> Sunspider and some form of plt testing would be sufficient I think.

Sunspider I can do.  How do I do plt testing?

Thanks for reviewing the patch.  :)


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.