Summary: | InspectorServer: Add Generic HTTP Request Class. Generalized HTTP Parsing. | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jocelyn Turcotte <jturcotte> | ||||||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Jocelyn Turcotte <jturcotte> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Enhancement | CC: | ap, dglazkov, hausmann, joepeck, rakuco, webkit.review.bot | ||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||
Bug Blocks: | 73094 | ||||||||||||||||||||
Attachments: |
|
Description
Jocelyn Turcotte
2011-11-24 13:13:26 PST
Created attachment 116537 [details]
Original New Part 3 patch
Created attachment 116538 [details]
Patch
Please ignore the link with the Part 3 patch, it was merged with Part 4 in bug #73093 and not with Part 2. Created attachment 134601 [details]
Patch
Rebased patch.
Created attachment 134607 [details]
Diff between WebSocketHandshake::readHTTPHeaders and parseHTTPHeader.
For review reference. (testing to see if this kind of stuff helps reviewing)
Since this spawned from one of my earlier patches on bug 51364, Alexey made some review comments that are still valid: <https://bugs.webkit.org/show_bug.cgi?id=51364#c45> - parseHTTPHeader assumes UTF-8 encoding (okay for WebSockets, not okay for generic HTTP) - possibility for localization of error messages Comment on attachment 134601 [details] Patch Attachment 134601 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12257001 New failing tests: http/tests/websocket/tests/hybi/fragmented-binary-frames.html http/tests/websocket/tests/hixie76/websocket-pending-activity.html http/tests/websocket/tests/hybi/handshake-challenge-randomness.html http/tests/websocket/tests/hybi/deflate-frame-invalid-parameter.html http/tests/websocket/tests/hybi/close-code-and-reason.html http/tests/websocket/tests/hybi/fragmented-control-frame.html http/tests/inspector/inspect-element.html accessibility/aria-disabled.html http/tests/websocket/tests/hybi/frame-lengths.html fast/loader/text-document-wrapping.html fast/frames/lots-of-objects.html http/tests/websocket/tests/hybi/fragmented-frames.html http/tests/websocket/tests/hybi/broken-utf8.html http/tests/websocket/tests/hybi/close-on-unload-reference-in-parent.html http/tests/websocket/tests/hybi/close-on-unload-and-force-gc.html http/tests/websocket/tests/hybi/compressed-control-frame.html http/tests/websocket/tests/hybi/close.html http/tests/websocket/tests/hybi/close-unref-websocket.html http/tests/websocket/tests/hybi/close-on-navigate-new-location.html http/tests/websocket/tests/hybi/bufferedAmount-after-close.html http/tests/websocket/tests/hybi/deflate-frame-comp-bit-onoff.html http/tests/websocket/tests/hybi/alert-in-event-handler.html http/tests/websocket/tests/hybi/cross-origin.html http/tests/websocket/tests/hybi/client-close.html http/tests/websocket/tests/hybi/deflate-frame-parameter.html http/tests/websocket/tests/hybi/bufferedAmount-after-close-in-busy.html http/tests/websocket/tests/hybi/close-on-unload.html http/tests/websocket/tests/hybi/extensions.html http/tests/websocket/tests/hybi/close-event.html Created attachment 134701 [details]
Archive of layout-test-results from ec2-cr-linux-04
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 134755 [details]
Patch
Fix the cr bot Sec-WebSocket-Extensions issue.
(In reply to comment #6) > Since this spawned from one of my earlier patches on bug 51364, Alexey made > some review comments that are still valid: > <https://bugs.webkit.org/show_bug.cgi?id=51364#c45> > > - parseHTTPHeader assumes UTF-8 encoding (okay for WebSockets, not okay for generic HTTP) > - possibility for localization of error messages The errors messages will probably not reach any real user ever in the case of the remote inspector and I probably won't spend time on it. I've opened bug #82714 for the header encoding, this patch just moved the code so I can address it separately. Thanks Comment on attachment 134755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134755&action=review > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:286 > + InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_document, m_identifier, *m_handshake->clientHandshakeRequest()); .get() ? > Source/WebCore/Modules/websockets/WebSocketHandshake.cpp:90 > +static String trimInputSample(const char* p, size_t len) length? > Source/WebCore/Modules/websockets/WebSocketHandshake.cpp:327 > + RefPtr<WebSocketHandshakeRequest> request = WebSocketHandshakeRequest::create("GET", m_url); > if (m_useHixie76Protocol) > - request.addHeaderField("Upgrade", "WebSocket"); > + request->addHeaderField("Upgrade", "WebSocket"); Isn't this the one that isnt supported? > Source/WebCore/platform/network/HTTPParsers.cpp:96 > +static const size_t maxInputSampleSize = 128; > +static String trimInputSample(const char* p, size_t length) > +{ You seem to have similar code elsewhere > Source/WebCore/platform/network/HTTPParsers.cpp:411 > +size_t parseHTTPRequestLine(const char* data, size_t length, String& failureReason, String& method, String& url, HttpVersion& httpVersion) why is this HttpVersion and not HTTPVersion? > Source/WebCore/platform/network/HTTPParsers.h:70 > +enum HttpVersion { Unknown, HTTP_1_0, HTTP_1_1 }; HTTPVersion? (In reply to comment #11) > > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:286 > > + InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_document, m_identifier, *m_handshake->clientHandshakeRequest()); > > .get() ? > The function takes a reference. > > Source/WebCore/Modules/websockets/WebSocketHandshake.cpp:90 > > +static String trimInputSample(const char* p, size_t len) > > length? > I think len is better for consistency in this file. > > Source/WebCore/Modules/websockets/WebSocketHandshake.cpp:327 > > + RefPtr<WebSocketHandshakeRequest> request = WebSocketHandshakeRequest::create("GET", m_url); > > if (m_useHixie76Protocol) > > - request.addHeaderField("Upgrade", "WebSocket"); > > + request->addHeaderField("Upgrade", "WebSocket"); > > Isn't this the one that isnt supported? > That's the client code, which still supports both. The old one is guarded by m_useHixie76Protocol. > > Source/WebCore/platform/network/HTTPParsers.cpp:96 > > +static const size_t maxInputSampleSize = 128; > > +static String trimInputSample(const char* p, size_t length) > > +{ > > You seem to have similar code elsewhere > Yes, I wasn't sure where to expose this, I don't want to pollute the namespace directly for such a simple method so I duplicated it. What would you suggest? Created attachment 135094 [details]
Patch
Rename HttpVersion to HTTPVersion
Committed r113024: <http://trac.webkit.org/changeset/113024> |