Last Comment Bug 748580 - websockets: omit close code (rather than code=1000) when no close code passed to close()
: websockets: omit close code (rather than code=1000) when no close code passed...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Jason Duell [:jduell] (needinfo? me)
:
Mentors:
: 753081 (view as bug list)
Depends on: 752776
Blocks: 753081
  Show dependency treegraph
 
Reported: 2012-04-24 15:37 PDT by Jason Duell [:jduell] (needinfo? me)
Modified: 2012-12-10 15:05 PST (History)
3 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1. Send empty close frame if no code given. (4.68 KB, patch)
2012-05-08 13:19 PDT, Jason Duell [:jduell] (needinfo? me)
mcmanus: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
Details | Diff | Review
mochi test (6.53 KB, patch)
2012-05-14 13:15 PDT, Jason Duell [:jduell] (needinfo? me)
mcmanus: review+
Details | Diff | Review

Description Jason Duell [:jduell] (needinfo? me) 2012-04-24 15:37:47 PDT
Takashi Toyoshima from the Chromium team notes that Chrome is omitting a code in this case.  The spec isn't crystal clear on it AFIACT, but I think omitting is more sensible (code 1005 doesn't really make sense w/o it), so I'm planning to implement it.
Comment 1 Jason Duell [:jduell] (needinfo? me) 2012-05-08 13:16:15 PDT
See also https://www.w3.org/Bugs/Public/show_bug.cgi?id=16703#c9 and Ian's response.
Comment 2 Jason Duell [:jduell] (needinfo? me) 2012-05-08 13:19:11 PDT
Created attachment 622111 [details] [diff] [review]
v1. Send empty close frame if no code given.

test will land in a separate bug, because it will depend on the pywebsockets mods I made in bug 752778, and so it may or may not land (and at different time).  I've tested this extensively, FWIW.
Comment 3 Jason Duell [:jduell] (needinfo? me) 2012-05-14 13:12:49 PDT
*** Bug 753081 has been marked as a duplicate of this bug. ***
Comment 4 Jason Duell [:jduell] (needinfo? me) 2012-05-14 13:15:54 PDT
Created attachment 623793 [details] [diff] [review]
mochi test

This is just the test from bug 753081 moved here with one minor tweak to the passive close handler (so we don't sent 1005 frames on the wire: a pywebsocket bug at some level--I've alerted them to it).
Comment 5 Patrick McManus [:mcmanus] 2012-05-17 07:14:20 PDT
Comment on attachment 622111 [details] [diff] [review]
v1. Send empty close frame if no code given.

The right thing to do here is ambiguous imo, but consistency across browsers is good and and the new way is just as good as the old way.
Comment 6 Jason Duell [:jduell] (needinfo? me) 2012-05-17 11:01:51 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/b65953b2b195
Comment 7 Jason Duell [:jduell] (needinfo? me) 2012-05-17 11:12:53 PDT
> The right thing to do here is ambiguous imo

The spec editors thinks it's not ambiguous, FWIW:

  https://www.w3.org/Bugs/Public/show_bug.cgi?id=16703#c10
Comment 8 Jason Duell [:jduell] (needinfo? me) 2012-05-17 11:17:57 PDT
Comment on attachment 622111 [details] [diff] [review]
v1. Send empty close frame if no code given.

Yet another low-risk Websocket spec compliance bug.  This one I'm only nomming (nom, nom!) because it's not quite as trivial as the others.  But still fairly low-risk IMO.

[Approval Request Comment]
User impact if declined:  we'll reply to servers with "OK" code (1000) when users actually meant to send no code (1005).  In most cases applications won't care.
Testing completed (on m-c, etc.):  lots of testing, including mochitest.
Risk to taking this patch (and alternatives if risky):  very low.
String or UUID changes made by this patch: none
Comment 9 Jason Duell [:jduell] (needinfo? me) 2012-05-17 11:18:47 PDT
only nomming /for beta/...
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-05-17 20:37:37 PDT
(In reply to Jason Duell (:jduell) from comment #6)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/b65953b2b195

https://hg.mozilla.org/mozilla-central/rev/b65953b2b195
Comment 11 Alex Keybl [:akeybl] 2012-05-18 15:40:55 PDT
Comment on attachment 622111 [details] [diff] [review]
v1. Send empty close frame if no code given.

[Triage Comment]
From the nomination, it's unclear what the motivation to fix this on our second to last beta is. WebSockets have been in the product since FF11, and we know of no sites behaving incorrectly because of this issue. Since FF14 appears to be affected, pre-approving for that branch since it still has time to bake this change prior to release.
Comment 12 Jason Duell [:jduell] (needinfo? me) 2012-05-18 21:21:09 PDT
This relies on pywebsocket changes, else aurora mochitest fails.  Requesting aurora in that bug
Comment 13 Jason Duell [:jduell] (needinfo? me) 2012-05-21 19:53:12 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc87dfeeeb88

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