import protocol buffer library

RESOLVED FIXED in mozilla20



7 years ago
6 years ago


(Reporter: mmc, Assigned: mmc)


(Blocks 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment, 4 obsolete attachments)

We need this library for bug 662819:

It's under BSD and Gerv confirms that it's compatible with MPL (license bug 810404). I'm pretty sure I only need the C++ bindings, not the python or Java ones, but other people might find those useful.

- Where does this belong in the tree?
- How much of the library do we want to import? The entire library, tests, docs, examples and all are 23M, C bindings are 9.3M, java and python are another 5.5M.
Assignee: nobody → mmc
Adding a bunch of people on jesup's recommendation.

Comment 2

7 years ago
9.3M!? That's exceedingly heavy, almost to the "nonstarted" point unless we don't actually need most of that and it's removed during link. What service is this interacting with? Can we use raw sockets instead of this library?
This is a serialization library, and I need it for a call to a Google API that returns a malware-or-not verdict based on file hashes. Just the serialization code compiles down to 532K.

If there's not a widespread need for this library, then I can strip out just the serialization code that I need -- but I thought I'd check to see if there's a better place for it to live first.
Eliminating tests/testdata gets the C++ source down to 1.8M.
Looks like there's not much interest in this library, so I'll plan to import just the parts I need as a static library under toolkit/components/downloads/protobuf following instructions here:
Please review
- the proposed location (toolkit/components/protobuf)
- toolkit/components/ and toolkit/components/build/
- toolkit/components/protobuf/README.txt

Tested by top level mach build and make -C toolkit/components export libs
Attachment #686303 - Flags: review?(khuey)
Comment on attachment 686303 [details] [diff] [review]
Initial import of protobuf-2.4.1

Review of attachment 686303 [details] [diff] [review]:

In the readme or somewhere you should make it clear what version of upstream we have.
Attachment #686303 - Flags: review?(khuey) → review+
Please include the changeset/rev that was imported in a file if at all possible (and date).

I'd prefer to include an update script such as used in netwerk/sctp/ or netwerk/srtp/
Added "This library was updated to version 2.4.1 as of 11/30/12" to the README, per khuey's request.

Randall, there were so many changes to the tree to trim it down after checking out the protobuf code that I don't think an update script is useful.
Attachment #686303 - Attachment is obsolete: true
Fixed title -- I hope that's all I missed. Patch applies cleanly to m-c.
Attachment #687161 - Attachment is obsolete: true
Flags: needinfo?(khuey)
You should have added 'r=khuey' to the title too.  I went ahead and did that.
Flags: needinfo?(khuey)
Target Milestone: --- → mozilla20

Comment 14

7 years ago
This broke the build on all platforms, so I backed it out.
I'm sorry about that -- the cause was a missing include that was installed locally and implicitly found on the include path. I am uninstalling everything now and will make sure it passes on try before trying again.
Fix missing include. TBPL:
Attachment #687180 - Attachment is obsolete: true
TPBL says I didn't break all the builds this time.
Keywords: checkin-needed
Last Resolved: 7 years ago
Resolution: --- → FIXED
Looks like protobuf ver. 2.5.0 was released 2 days ago:

Should we get a tracking bug on upgrading? (not sure how frequently we want/need to upgrade)
That's probably a good idea, but it's going to be low-ish priority for me since I haven't even checked in the initial caller(?!) and no one else is using it yet. I'm inclined to wait for 2.5.1 for bugs to shake out.


6 years ago
Blocks: 837199

Comment 25

6 years ago
I'm currently reviewing bug 837199, that uses this library to decode content that
is provided by a remote service whose URL can be reconfigured from about:config.
So, we're potentially feeding arbitrary data to a native C++ library.

I'm not familiar with the security review process, or how we deal with external
libraries, but it looks like we need at least to be notified in case any
vulnerability is found so that we can update the library if needed.

Is there any process for doing this?
Hi Paolo,

I pinged bsmith and dveditz on #security to ask about this, but in the meantime I am subscribed to the discussion list (not very scalable, I know!)

Pretty much every 3rd party library in about:license is subject to the same caveats, but as far as I know we don't really have a master plan for dealing with external vulnerabilities other than someone is paying attention to vulnerability reports. In general security team pays close attention to plugin security and I know they monitor, and since technically I am part of security team I guess they monitor protocol buffer announcements as well :)

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