[webkit-reviews] review denied: [Bug 68560] [Qt] HTTP header injection via QWebPage::userAgentForUrl() : [Attachment 119916] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Jarred Nicholls
<jarred at webkit.org>'s request for review:
Bug 68560: [Qt] HTTP header injection via QWebPage::userAgentForUrl()
https://bugs.webkit.org/show_bug.cgi?id=68560

Attachment 119916: Patch
https://bugs.webkit.org/attachment.cgi?id=119916&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119916&action=review


I think that's a very good solution to the problem. I think this can be written
more efficiently though and there's a style nitpick :)

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:739
> +	   return
m_webFrame->page()->userAgentForUrl(url).replace(QLatin1String("\n"),
QString()).replace(QLatin1String("\r"), QString());

I think it is better to use remove() instead of replace():

    return
m_webFrame->page()->userAgentForUrl(url).remove(QLatin1Char('\n')).remove(QLati
n1Char('\r'));

I think in the common case of _no_ newlines/linefeeds being present, the simple
iteration that QString::remove(QChar) does is faster
than creating temporary QStrings and firing up the QStringMatcher machinery.