Closed Bug 1313385 Opened 4 years ago Closed 4 years ago
Socket Impl::Web Socket Impl does not initialise m Secure
This is dynamically observable in our test suites. STR to follow. Setting as sec-sensitive because it seems to me to be ungood to be confused about whether a connection is secure or not. Please declassify as appropriate.
Looking at dom/base/WebSocket.cpp, it seems that class WebSocketImpl has a |bool mSecure|, but |explicit WebSocketImpl(WebSocket* aWebSocket)| does not give an initial value for it. I am hypothesising that this is the cause of the reported complaint.
Attachment #8805213 - Flags: review?(kyle) → review+
(In reply to Andrea Marchesini [:baku] from comment #3) > Created attachment 8805213 [details] [diff] [review] > ws.patch Verified as fixing the complaint. The test is devtools/server/tests/mochitest/test_websocket-server.html FTR, command line I used is DISPLAY=:1.0 G_SLICE=always-malloc ./mach mochitest -f chrome --debugger=v312BRANCH \ --debugger-args="--fair-sched=yes --smc-check=all-non-file \ --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc-may2015.supp --error-limit=no \ --trace-children=yes \ --child-silent-after-fork=yes --trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep,*/bin/java \ --num-transtab-sectors=24 --tool=memcheck --freelist-vol=300000000 --redzone-size=64 \ --gen-suppressions=no --px-default=allregs-at-mem-access \ --px-file-backed=unwindregs-at-mem-access --stats=no --show-mismatched-frees=no \ --fullpath-after=/MOZ/ --num-callers=20 --track-origins=no" --keep-open=no \ devtools/server/tests/mochitest/test_websocket-server.html 2>&1 | tee spew-01-06-mc
Hi :dveditz, Since 51 is also affected, is this worth uplifting to 51?
sure, this is a trivially safe fix to uplift to 51 (and ESR for that matter).
Hi :baku, could you please nominate this uplift to Aurora51 & ESR?
Comment on attachment 8805213 [details] [diff] [review] ws.patch Approval Request Comment [Feature/regressing bug #]: WebSocket ported to webIDL [User impact if declined]: mSecure is not initialized in the CTOR. In theory until the initialization is completed in other methods, this value is unset. [Describe test coverage new/current, TreeHerder]: none [Risks and why]: none [String/UUID change made/needed]: none
Attachment #8805213 - Flags: approval-mozilla-aurora?
Comment on attachment 8805213 [details] [diff] [review] ws.patch Fix a sec-moderate. Take it in 51 aurora.
Attachment #8805213 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This may as well wait for the ESR that will go out with 51 (45.7.0).
You need to log in before you can comment on or make changes to this bug.