Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(340)

Issue 3010363002: Implement GetChain for OpenSSLCertificate.

Created:
3 years, 3 months ago by jiancui
Modified:
3 years, 2 months ago
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Implement GetChain for OpenSSLCertificate. BUG=8289

Patch Set 1 #

Total comments: 6

Patch Set 2 : Remove clang-format changes and fix lint errors. #

Patch Set 3 : Adding limit to chain size. Remove debug logging. #

Total comments: 6

Patch Set 4 : Adding unit tests and clean up. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+362 lines, -27 lines) Patch
M rtc_base/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M rtc_base/opensslidentity.h View 1 2 3 6 chunks +10 lines, -7 lines 0 comments Download
M rtc_base/opensslidentity.cc View 1 2 3 8 chunks +77 lines, -12 lines 0 comments Download
A rtc_base/opensslidentity_unittest.cc View 1 2 3 1 chunk +146 lines, -0 lines 0 comments Download
M rtc_base/opensslstreamadapter.cc View 1 2 3 2 chunks +7 lines, -8 lines 0 comments Download
M rtc_base/sslstreamadapter_unittest.cc View 1 2 3 2 chunks +121 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
Taylor Brandstetter
Can you file a webrtc bug and describe what problem this is solving, in general ...
3 years, 2 months ago (2017-09-26 00:57:26 UTC) #3
pthatcher1
https://codereview.webrtc.org/3010363002/diff/1/webrtc/rtc_base/opensslidentity.cc File webrtc/rtc_base/opensslidentity.cc (right): https://codereview.webrtc.org/3010363002/diff/1/webrtc/rtc_base/opensslidentity.cc#newcode287 webrtc/rtc_base/opensslidentity.cc:287: LOG(LS_INFO) << "leaf_cert:" << x509_->name; On 2017/09/26 00:57:26, Taylor ...
3 years, 2 months ago (2017-09-26 20:55:07 UTC) #5
jiancui
On 2017/09/26 00:57:26, Taylor Brandstetter wrote: > Can you file a webrtc bug and describe ...
3 years, 2 months ago (2017-09-26 21:53:39 UTC) #7
jiancui
On 2017/09/26 20:55:07, pthatcher1 wrote: > https://codereview.webrtc.org/3010363002/diff/1/webrtc/rtc_base/opensslidentity.cc > File webrtc/rtc_base/opensslidentity.cc (right): > > https://codereview.webrtc.org/3010363002/diff/1/webrtc/rtc_base/opensslidentity.cc#newcode287 > ...
3 years, 2 months ago (2017-09-26 22:40:34 UTC) #8
davidben_webrtc
> > Also: I know it's a lot to ask for someone not familiar with ...
3 years, 2 months ago (2017-09-26 23:21:46 UTC) #9
jiancui
On 2017/09/26 23:21:46, davidben_webrtc wrote: > > > Also: I know it's a lot to ...
3 years, 2 months ago (2017-09-27 00:02:49 UTC) #10
davidben_webrtc
I've uploaded https://webrtc-review.googlesource.com/c/src/+/4361 and https://webrtc-review.googlesource.com/c/src/+/4581 which should resolve the more complicated of my comments. Once ...
3 years, 2 months ago (2017-09-28 21:18:34 UTC) #11
jiancui
3 years, 2 months ago (2017-09-28 22:05:38 UTC) #12
On 2017/09/28 21:18:34, davidben_webrtc wrote:
> I've uploaded https://webrtc-review.googlesource.com/c/src/+/4361 and
> https://webrtc-review.googlesource.com/c/src/+/4581 which should resolve the
> more complicated of my comments. Once those go in, you should have a much more
> solid base to do things on.
> 
> By the way, I think you may have an old checkout (I did too until recently). I
> think WebRTC master has since moved to Gerrit and also removed the webrtc/ in
> the path. You may need to recreate your patch.

Thanks, I just upload another patch set. I'll merge once yours are in. I think
my current set of change don't have much conflict with yours.

In the mean time, I added some unit tests. In the middle of adding UT, I think
there may need some semantic changes in OpenSSLCertificate api. Please take a
look.

Powered by Google App Engine
This is Rietveld 408576698