1332213 - Support for dumping failed update for v2

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[Henry Chang [:hchang]]

Dumping failed update was supported by Bug 1310142. However, it only works for v4 since |mPending| will be cut [1] while it's being parsed. 

In order to debug Bug 1304602, I'd like to duplicate the crash on my own with
the original database and update data collected so we have to support v2 as well.

[1] https://dxr.mozilla.org/mozilla-central/rev/96cb95af530477edb66ae48d98c18533476e57bb/toolkit/components/url-classifier/ProtocolParser.cpp#364

Comment 1

[Henry Chang [:hchang]]

Attachment: Bug 1332213 - Store the update stream duplicately in v2 protocol parser. .

Review commit: https://reviewboard.mozilla.org/r/105728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105728/

Comment 2

[François Marier [:francois]]

Comment on attachment 8828255 [details]
Bug 1332213 - Store the update stream duplicately in v2 protocol parser. .

https://reviewboard.mozilla.org/r/105728/#review106904

::: toolkit/components/url-classifier/ProtocolParser.h:106
(Diff revision 1)
>    virtual const nsTArray<ForwardedUpdate> &Forwards() const override { return mForwards; }
>    virtual bool ResetRequested() override { return mResetRequested; }
>  
> +#ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
> +  // Unfortunately we have to override to return mRawUpdate which
> +  // will never be cut during parsing.

To make the comment clearer, I would suggest:

    // will not be modified during the parsing, unlike mPending.

::: toolkit/components/url-classifier/ProtocolParser.h:169
(Diff revision 1)
>  
>    // Updates to apply to the current table being parsed.
>    TableUpdateV2 *mTableUpdate;
> +
> +#ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
> +  nsCString mRawUpdate; // Equal to mPending but will never be cut.

To make the comment clearer, I would suggest:

    // Keep a copy of mPending before it's processed.

Comment 3

[Henry Chang [:hchang]]

Comment on attachment 8828255 [details]
Bug 1332213 - Store the update stream duplicately in v2 protocol parser. .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105728/diff/1-2/

Comment 4

[Henry Chang [:hchang]]

Comment on attachment 8828255 [details]
Bug 1332213 - Store the update stream duplicately in v2 protocol parser. .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105728/diff/2-3/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c8fc27b018c
Store the update stream duplicately in v2 protocol parser. r=francois.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9c8fc27b018c