Last Comment Bug 687243 - Firefox/WebSockets uses non-minimal payload length encoding
: Firefox/WebSockets uses non-minimal payload length encoding
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: 9 Branch
: x86 Windows 7
: -- enhancement (vote)
: mozilla10
Assigned To: Patrick McManus [:mcmanus]
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-17 05:09 PDT by Tobias Oberstein
Modified: 2011-12-16 14:24 PST (History)
8 users (show)
Ms2ger: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1 (1.15 KB, patch)
2011-09-18 11:20 PDT, Patrick McManus [:mcmanus]
jduell.mcbugs: review+
Details | Diff | Splinter Review

Description Tobias Oberstein 2011-09-17 05:09:20 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:9.0a1) Gecko/20110916 Firefox/9.0a1
Build ID: 20110916071142

Steps to reproduce:

Send WS text message with payload length = 65535.

See test case 1.1.6

http://www.tavendo.de/autobahn/testsuite/report/tmp1/index.html


Actual results:

Firefox sends a WS text frame starting with

81ff000000000000ffff

It uses the 7+64 bits encoding.


Expected results:

Firefox should have sent a frame starting with

81feffff

(when payload isn't fragmented)

This is the minimal length encoding in this case (7+16 bits).

The WS draft 14 says:

"""
5.2.  Base Framing Protocol

Multibyte length quantities are expressed in network byte order.  Note that
in all case the minimal number of bytes MUST be used to encode the
length, for example the length of a 124 byte long string can't be
encoded as the sequence 126, 0, 124.
"""

Note that the test cases which test payload length 125, 126, 127 and 128 (cases 1.1.2 - 1.1.5) work as expected.

Note that up till now, the test suite I ran until recently a) did not detect overlong payload length encoding and b) the (now failed) case 1.1.6 was doing a payload of length 65536, and thus missed the opportunity to catch these issues anyway. Both of these things are fixed only on Autobahn HEAD (yet unreleased) .. will be in Autobahn 0.4.3.
Comment 1 Patrick McManus [:mcmanus] 2011-09-18 10:25:51 PDT
> The WS draft 14 says:
> 
> """

Hi Tomas, thanks as always for the detailed information. I've been on the road for the last week and haven't had a chance to really dig into your other report (about various utf-8 sequences), but I will do so this week.

As for this issue, Firefox 9 imlpements draft 10, not draft 14. Draft 10 does not have the requirement you cite (as far as I can tell, anyhow.). As such this is a desirable enhancement request and not a bug for this implementation.

Thanks!
-Pat
Comment 2 Patrick McManus [:mcmanus] 2011-09-18 10:44:43 PDT
(In reply to Patrick McManus from comment #1)
> Hi Tomas,

Tobias, I'm sorry.
Comment 3 Patrick McManus [:mcmanus] 2011-09-18 11:20:07 PDT
Created attachment 560800 [details] [diff] [review]
patch 1
Comment 4 Tobias Oberstein 2011-09-18 15:23:38 PDT
Hi Patrick,

yep, it was only introduced draft >10, I think only in draft 14, and thus not a bug exactly. Anyway, nice patch .. clearly laid out;)
Comment 5 Mozilla RelEng Bot 2011-09-18 15:50:57 PDT
Try run for 5c4e66ab5a60 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5c4e66ab5a60
Results (out of 10 total builds):
    success: 10
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mcmanus@ducksong.com-5c4e66ab5a60
Comment 6 Ed Morley [:emorley] 2011-09-27 06:15:56 PDT
Changing milestone to mozilla10, since the last inbound to m-c merge has already happened, sorry.
Comment 7 Marco Bonardo [::mak] 2011-09-28 01:56:04 PDT
https://hg.mozilla.org/mozilla-central/rev/37843077ff3d
Comment 8 Eric Shepherd [:sheppy] 2011-12-14 09:16:32 PST
This looks like a pretty simple bug fix rather than an issue that requires documentation... any thoughts?
Comment 9 Jason Duell [:jduell] (needinfo me) 2011-12-16 14:24:00 PST
agreed--no doc needed for this.

Note You need to log in before you can comment on or make changes to this bug.