Update csd.proto to support login reputation

RESOLVED FIXED in Firefox 58

Status

()

Toolkit
Safe Browsing
P2
normal
RESOLVED FIXED
25 days ago
20 days ago

People

(Reporter: dimi, Assigned: dimi)

Tracking

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

(Whiteboard: pwphish-prep )

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

25 days ago
Login reputation will send lookup request to remote server to find out if a password field belongs to a phishing site.

The lookup request/response is defined in latest csd.proto.
Comment hidden (mozreview-request)

Comment 2

24 days ago
mozreview-review
Comment on attachment 8922223 [details]
Bug 1411861 - Update csd.proto to support login reputation.

https://reviewboard.mozilla.org/r/193244/#review198678

::: commit-message-508cf:2
(Diff revision 1)
> +Bug 1411861 - Update csd.proto to support login reputation. r?francois
> +

I would suggest adding a link to the upstream source for the file so that we can easily refer to it in the future if we need to:

https://cs.chromium.org/chromium/src/components/safe_browsing/proto/csd.proto?rcl=9f9b74758ad17f40ee048c09777bddee513d5cd8

::: toolkit/components/reputationservice/chromium/chrome/common/safe_browsing/csd.pb.cc:5
(Diff revision 1)
>  // Generated by the protocol buffer compiler.  DO NOT EDIT!
> -// source: csd.proto
> +// source: chromium/chrome/common/safe_browsing/csd.proto
>  
>  #define INTERNAL_SUPPRESS_PROTOBUF_FIELD_DEPRECATION
> -#include "csd.pb.h"
> +#include "chromium/chrome/common/safe_browsing/csd.pb.h"

That line shouldn't change.

The reason why it did is because I forgot to update the regeneration script here the last time I updated this.

Basically, we need to `cd` into the directory where the `.proto` file is before running `protoc`: https://searchfox.org/mozilla-central/source/toolkit/components/protobuf/regenerate_cpp_files.sh#17-18

If you fix the script and re-run it, the generated files should look right.

Given the size of these changes, it's also probably worth splitting the changes to the script in a separate commit that would come before the big one.
Attachment #8922223 - Flags: review?(francois) → review-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

23 days ago
mozreview-review
Comment on attachment 8922223 [details]
Bug 1411861 - Update csd.proto to support login reputation.

https://reviewboard.mozilla.org/r/193244/#review199128
Attachment #8922223 - Flags: review?(francois) → review+

Comment 8

23 days ago
mozreview-review
Comment on attachment 8922625 [details]
Bug 1411861 - Regenerate csd.pb.cc & csd.pb.h.

https://reviewboard.mozilla.org/r/193742/#review199132

::: commit-message-1aec2:1
(Diff revision 2)
> +Bug 1411861 - Update csd.pb.cc & csd.pb.h. r?francois

nit: I would say "regenerate" instead of "update" to make it clear that it's a mechanical process
Attachment #8922625 - Flags: review?(francois) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 12

20 days ago
Pushed by dlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a22b9c150733
Update csd.proto to support login reputation. r=francois
https://hg.mozilla.org/integration/autoland/rev/5fc8269c1058
Regenerate csd.pb.cc & csd.pb.h. r=francois
https://hg.mozilla.org/mozilla-central/rev/a22b9c150733
https://hg.mozilla.org/mozilla-central/rev/5fc8269c1058
Status: ASSIGNED → RESOLVED
Last Resolved: 20 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.