Chromium Code Reviews
Help | Chromium Project | Sign in
(690)

Issue 2812064: Revert 52799 - Add support for parsing certificate formats other than raw, DE... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 9 months ago by Ryan Sleevi
Modified:
2 years, 11 months ago
CC:
chromium-reviews_chromium.org, pam+watch_chromium.org, John Grabowski, cbentzel+watch_chromium.org, darin-cc_chromium.org, PaweĊ‚ Hajdan Jr.
Visibility:
Public.

Description

Revert 52799 - Add support for parsing certificate formats other than raw, DER-encoded certificates - specifically formats that represent collections of certificates. The certificate format can now be specified as an explicit format, or as a bit-mask of formats that are acceptable/expected, with the first parsable format winning.

This is one half of a commit to address BUG #37142, with the second half involving connecting this through the X509UserCertHandler and the actual UI.

R=wtc
BUG=37142
TEST=X509CertificateParseTest* and PEMTokenizerTest.*

Review URL: http://codereview.chromium.org/2819018

TBR=rsleevi@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52801

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -1035 lines) Lint Patch
D net/base/pem_tokenizer.h View 1 chunk +0 lines, -79 lines 0 comments 0 errors Download
D net/base/pem_tokenizer.cc View 1 chunk +0 lines, -95 lines 0 comments 0 errors Download
D net/base/pem_tokenizer_unittest.cc View 1 chunk +0 lines, -169 lines 0 comments 0 errors Download
M net/base/x509_certificate.h View 5 chunks +1 line, -37 lines 0 comments 0 errors Download
M net/base/x509_certificate.cc View 3 chunks +0 lines, -89 lines 0 comments 0 errors Download
M net/base/x509_certificate_mac.cc View 3 chunks +2 lines, -78 lines 0 comments 0 errors Download
M net/base/x509_certificate_nss.cc View 3 chunks +8 lines, -63 lines 0 comments 0 errors Download
M net/base/x509_certificate_unittest.cc View 5 chunks +8 lines, -154 lines 1 comment 0 errors Download
M net/base/x509_certificate_win.cc View 2 chunks +0 lines, -69 lines 0 comments 0 errors Download
D net/data/ssl/certificates/google.binary.p7b View 1 chunk +0 lines, -32 lines 0 comments 0 errors Download
D net/data/ssl/certificates/google.chain.pem View 1 chunk +0 lines, -38 lines 0 comments 0 errors Download
D net/data/ssl/certificates/google.pem_cert.p7b View 1 chunk +0 lines, -37 lines 0 comments 0 errors Download
D net/data/ssl/certificates/google.pem_pkcs7.p7b View 1 chunk +0 lines, -37 lines 0 comments 0 errors Download
D net/data/ssl/certificates/google.single.der View 1 chunk +0 lines, -17 lines 0 comments 0 errors Download
D net/data/ssl/certificates/google.single.pem View 1 chunk +0 lines, -19 lines 0 comments 0 errors Download
D net/data/ssl/certificates/thawte.single.pem View 1 chunk +0 lines, -19 lines 0 comments 0 errors Download
M net/net.gyp View 2 chunks +0 lines, -3 lines 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 3
Ryan Sleevi
3 years, 9 months ago #1
wtc
When you revert a CL, please add a comment to note why you had to ...
3 years, 9 months ago #2
rsleevi-old
3 years, 9 months ago #3
On 2010/07/17 13:28:58, wtc wrote:
> When you revert a CL, please add a comment to note why
> you had to revert it.
> 
> The CL you reverted broke the Mac builds.  I found some
> compiler warnings in x509_certificate_unittest.cc that
> were treated as errors.  I don't know what those warnings
> means.  I hope you can figure out how to fix them.
> 
> http://codereview.chromium.org/2812064/diff/1/9
> File net/base/x509_certificate_unittest.cc (left):
> 
> http://codereview.chromium.org/2812064/diff/1/9#oldcode669
> net/base/x509_certificate_unittest.cc:669: for (size_t j = 0; j < 20; ++j)
> The constant 20 should be replaced by
> sizeof(actual_fingerprint.data).

Yeah, this was my first time using drover, and it didn't like my cygwin
environment that I was running from (Specifically, it wasn't able to find fi,
which would have allowed me to edit the message). I was given the option of futz
with it, or just revert.

I've fixed the issue in the original CL. for whatever reason, the OS X 10.5 gcc
4.2.1, unlike all the other boxes/compilers, didn't like some of the data being
in an anonymous namespace. The simple fix was to remove the anonymous namespace
qualifier in the x509_certificate_unittests. This doesn't have any functional
effect, even to linking, since all of the implementation is contained within
that file and only used by the unit tests.

This non-use of anon was consistant with the other places in code using
instantiated tests, so it seems correct. I was just waiting to reland until I
had a chance to babysit the tree again.

Trybots compile it fine (though they fail the tests, because the trybots don't
like binary files), so it's all good.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6