Closed Bug 812238 Opened 12 years ago Closed 12 years ago

import protocol buffer library

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla20

People

(Reporter: mmc, Assigned: mmc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

Initial import of protobuf-2.4.1
472.09 KB, patch
Details | Diff | Splinter Review 
We need this library for bug 662819:

http://code.google.com/p/protobuf/

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.
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:

https://developer.mozilla.org/en-US/docs/How_Mozilla%27s_build_system_works
Attached patch Initial import of protobuf-2.4.1 (obsolete) — — Splinter Review 
Please review
- the proposed location (toolkit/components/protobuf)
- toolkit/components/Makefile.in and toolkit/components/build/Makefile.in
- 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/update_sctp.sh or netwerk/srtp/update_srtp.sh
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.

https://hg.mozilla.org/integration/mozilla-inbound/rev/98b22b52150e
Flags: needinfo?(khuey)
Target Milestone: --- → mozilla20
This broke the build on all platforms, so I backed it out.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4f180373335
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: https://tbpl.mozilla.org/?tree=Try&rev=01e11a0f8653
Attachment #687180 - Attachment is obsolete: true
TPBL says I didn't break all the builds this time.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/492eb4f0a70c
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Looks like protobuf ver. 2.5.0 was released 2 days ago:
 https://code.google.com/p/protobuf/downloads/list

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.
Blocks: 837199
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 http://cve.mitre.org/index.html, and since technically I am part of security team I guess they monitor protocol buffer announcements as well :)

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

Attachment

General

Creator:
Created:
Updated:
Size: