[Webkit-unassigned] [Bug 32249] WebSocket test server handshake is not strict enough

[bugzilla-daemon at webkit.org]

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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #44524|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #7 from Alexey Proskuryakov <ap at webkit.org>  2009-12-09 08:46:44 PST ---
(From update of attachment 44524)
> +        "--strict",

I don't think this should be an option - why would we want to encourage sloppy
implementations?

> +_FIRST_FIVE_LINES = map(re.compile, [
> +    r'^GET [\S]+ HTTP/1.1\r\n$',

The spec also says that path must begin with a slash.

> +_SIXTH_AND_LATER = re.compile(
> +    r'^'
> +    r'(WebSocket-Protocol: [\x20-\x7e]+\r\n)?'
> +    r'([Cc][Oo][Oo][Kk][Ii][Ee]2?:[^\r]*(\r\n[ \t][^\r]*)*\r\n)?'
> +    r'\r\n')

The Cookie header must also be in the restricted WebSocket form. The spec says
its semantics matches HTTP, but doesn't say anything special about syntax.

Does this allow having both Cookie and Cookie2 headers in a single request?

>      for c in protocol:
> -        if not 0x21 <= ord(c) <= 0x7e:
> +        if not 0x20 <= ord(c) <= 0x7e:
>              raise HandshakeError('Illegal character in protocol: %r' % c)

I'm confused - bug 32266 has already introduced a test verifying that space is
allowed in protocol name. How could that possibly work before this change?

> +_STRICTLY_GOOD_REQUESTS = (
<...>
> +    (  # Cookie with continuation lines

This shouldn't be allowed - all handshake header fields are subject to
WebSocket syntax.

> +_NOT_STRICTLY_GOOD_REQUESTS = (
> +    (  # Extra space after GET
> +        'GET  /demo HTTP/1.1\r\n',
> +        'upgrade: WebSocket\r\n',

Upgrade doesn't have a correct case here.

Please also test path without a slash at the beginning.

> +    (  # Origin comes before Host

Please also test incorrect order of Upgrade and Connection

> +    (  # Unknow header

Typo: unknown.

> +# vi:sts=4 sw=4 et

We don't normally allow modelines in WebKit sources.

Looks good, thanks for tackling this! I can't deeply review Python code, but it
looks reasonable to me, too.

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