The permessage compression extension requires "method" parameter. The value of the parameter is defined at http://tools.ietf.org/html/draft-ietf-hybi-permessage-compression-01#section-3. Here is the ABNFs. requested-method-list = 1#method-desc method-desc = method-name *(";" method-param) method-name = token method-param = token ["=" (token | quoted-string)] This is almost the same as how the value of Sec-WebSocket-Extensions parsed. We can share the code of ExtensionParser to parse "method" parameter value of permessage compression. I'm going to factor out ExtensionParameter from WebSocketExtensionDispatcher and put it on its own file so that we can reuse the class.
Created attachment 167238 [details] Patch
Comment on attachment 167238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167238&action=review Mostly looks good. Please fix the nits before landing. > Source/WebCore/Modules/websockets/WebSocketExtensionParser.cpp:94 > + m_currentToken = String::fromUTF8(buffer.data(), buffer.size()); (I know this code is moved from another file without modification, but I'm just curious. If you need to change the code, you can do it in a later patch.) 1. Is there any normative reference that says the content of a quoted string must be decoded as UTF-8? 2. What happens if the quoted string contains invalid UTF-8 sequence? > Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:37 > +#include <wtf/text/StringHash.h> Is this necessary? > Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:56 > + // The following member functions basically follow the grammer defined nit: "grammar"
Created attachment 167260 [details] Patch for landing
Comment on attachment 167238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167238&action=review Thank you for review! >> Source/WebCore/Modules/websockets/WebSocketExtensionParser.cpp:94 >> + m_currentToken = String::fromUTF8(buffer.data(), buffer.size()); > > (I know this code is moved from another file without modification, but I'm just curious. > If you need to change the code, you can do it in a later patch.) > > 1. Is there any normative reference that says the content of a quoted string must be > decoded as UTF-8? > 2. What happens if the quoted string contains invalid UTF-8 sequence? I found my old comment on that (https://bugs.webkit.org/show_bug.cgi?id=78079#c20). I used it because WebSocketHandshake class uses it to parse header fields. If the quoted string contains invalid UtF-8 sequence, m_currentToken could be an empty string. It might be better to use CString. I'd like to do it as a separate patch because it affects other files. >> Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:37 >> +#include <wtf/text/StringHash.h> > > Is this necessary? I thought it's unnecessary, but when I removed it, I got following error. ../../third_party/WebKit/Source/WebCore/Modules/websockets/WebSocketExtensionParser.cpp:132: instantiated from here ../../third_party/WebKit/Source/WTF/wtf/HashTable.h:844: error: incomplete type 'WTF::StringHash' used in nested name specifier ... >> Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:56 >> + // The following member functions basically follow the grammer defined > > nit: "grammar" Done. Thanks for pointing this out.
Comment on attachment 167260 [details] Patch for landing Clearing flags on attachment: 167260 Committed r130471: <http://trac.webkit.org/changeset/130471>
All reviewed patches have been landed. Closing bug.