[Webkit-unassigned] [Bug 65835] Need a way to selectively use hixie-76 for websocket connections depending on destination and/or origin

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 11 05:04:02 PDT 2011


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





--- Comment #10 from Denis Lagno <dilmah at chromium.org>  2011-08-11 05:04:03 PST ---

(In reply to comment #8)
> (From update of attachment 103506 [details])
> 1. Normally, no patches can be landed without a ChangeLog: <http://www.webkit.org/coding/contributing.html>.

done

> 3. I think that the patch has a wrong root (it should be above Source).

done

> > WebCore/page/Settings.h:445
> > +        void addHixie76WebSocketProtocolException(KURL url);
> 
> Should be "const KURL&", and argument should be unnamed, as it doesn't add any information.

argument name fixed.  As for type: if passed as const reference then this method must copy its argument into temporary local immediately upon entering method.  It is equivalent to just passing by value -- and it is simpler and more straightforward.

> > WebCore/page/Settings.h:447
> > +        bool useHixie76WebSocketProtocol(KURL url);
> 
> Ditto.

done

> > WebCore/page/Settings.h:575
> > +        HashMap<String, bool> m_hixie76WebSocketProtocolExceptionList;
> 
> Why not HashSet?

done

> > WebCore/page/Settings.cpp:802
> > +    url.setPath(String());
> > +    url.setQuery(String());
> > +    url.setUser(String());
> > +    url.setPass(String());
> > +    url.removeFragmentIdentifier();
> 
> Perhaps you want SecurityOrigin(url).toString()? Not sure - this kind of exception is not pretty either way.

SecurityOrigin is too heavyweight for this purpose.  Also its usage here misleads reader because we do not really treat url as security origin (also I doubt if security origin can be of "ws://" scheme).

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



More information about the webkit-unassigned mailing list