Last Comment Bug 729640 - Protocol parsing is broken in UrlClassifier
: Protocol parsing is broken in UrlClassifier
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Safe Browsing (show other bugs)
: 13 Branch
: All All
: -- normal (vote)
: Firefox 17
Assigned To: Gian-Carlo Pascutto [:gcp]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-22 11:33 PST by Gian-Carlo Pascutto [:gcp]
Modified: 2014-05-27 12:25 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch 1. Fix the assertion check and message. (1.32 KB, patch)
2012-02-23 08:36 PST, Gian-Carlo Pascutto [:gcp]
dcamp: review+
Details | Diff | Review
Patch 1. Backout (1.30 KB, patch)
2012-04-19 09:18 PDT, Gian-Carlo Pascutto [:gcp]
akeybl: approval‑mozilla‑aurora+
mark.finkle: approval‑mozilla‑central+
Details | Diff | Review

Description Gian-Carlo Pascutto [:gcp] 2012-02-22 11:33:28 PST
Occasionally an update will trigger the following warning:

2012-02-22 19:20:16.406178 UTC - -655362304[7f47d9852df0]: nsUrlClassifierDBServiceWorker::BeginStream
2012-02-22 19:20:16.406187 UTC - -655362304[7f47d9852df0]: Expecting MAC in this stream
2012-02-22 19:20:16.406829 UTC - -655362304[7f47d9852df0]: ###!!! ASSERTION: ProcessHostSub should only be called for prefix hashes.: 'mChunkState.hashSize == PREFIX_SIZE', file /home/morbo/hg/mozilla-central/toolkit/components/url-classifier/ProtocolParser.cpp, line 706
2012-02-22 19:20:16.407616 UTC - -22583520[7f47fd85b150]: OnDataAvailable (14180 bytes)
2012-02-22 19:20:16.407791 UTC - -22583520[7f47fd85b150]: OnDataAvailable (1418 bytes)
2012-02-22 19:20:16.410594 UTC - -22583520[7f47fd85b150]: OnDataAvailable (2836 bytes)
Comment 1 Gian-Carlo Pascutto [:gcp] 2012-02-22 12:24:02 PST
Okay, this is a really stupid bug. It's only the assertion that is wrong. It's sitting in ProcessHostSubComplete not ProcessHostSub, but was copypasted from there without changing PREFIX_SIZE into COMPLETE_SIZE.

What's slightly surprising to me is that there are actual SubCompletes out there being used in the wire protocol.
Comment 2 Gian-Carlo Pascutto [:gcp] 2012-02-23 08:36:35 PST
Created attachment 600034 [details] [diff] [review]
Patch 1. Fix the assertion check and message.
Comment 3 Gian-Carlo Pascutto [:gcp] 2012-02-23 09:02:57 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8bf3795b851
Comment 4 Richard Newman [:rnewman] 2012-02-23 18:43:50 PST
https://hg.mozilla.org/mozilla-central/rev/f8bf3795b851
Comment 5 Gian-Carlo Pascutto [:gcp] 2012-04-19 09:18:06 PDT
Created attachment 616609 [details] [diff] [review]
Patch 1. Backout

[Approval Request Comment]
Backout due to bug 744993:

a01cf079ee0b Bug 730247
1a6d008acb4f Bug 729928
f8bf3795b851 Bug 729640
35bf0d62cc30 Bug 726002
a010dcf1a973 Bug 726002
e9291f227d63 Bug 725597
db52b4916cde Bug 673470
173f90d397a8 Bug 673470
Comment 6 Gian-Carlo Pascutto [:gcp] 2012-04-19 22:49:53 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/01d2f5d68c15
Comment 7 :Ehsan Akhgari (out sick) 2012-04-20 10:44:11 PDT
https://hg.mozilla.org/mozilla-central/rev/01d2f5d68c15
Comment 8 Alex Keybl [:akeybl] 2012-04-24 07:11:29 PDT
Comment on attachment 616609 [details] [diff] [review]
Patch 1. Backout

Sorry - thought this bug was reopened because the backout was backed out. My mistake. Approved for Aurora 13 (or Beta 13 if the merge occurs before we land).
Comment 9 Gian-Carlo Pascutto [:gcp] 2012-05-01 06:41:44 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/8c4ce03cc108
Comment 10 Gian-Carlo Pascutto [:gcp] 2012-08-15 00:15:31 PDT
Relanding after fixes in bug 673470 to fix bug 744993.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f125c23208c
Comment 11 Ed Morley [:emorley] 2012-08-15 09:47:34 PDT
https://hg.mozilla.org/mozilla-central/rev/0f125c23208c

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