Sailesh Mukil has posted comments on this change.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with 
Kudu
......................................................................


Patch Set 4:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/7662/2//COMMIT_MSG
Commit Message:

PS2, Line 15: 
> nit: please make the string <= 72 characters long
Done


PS2, Line 18: through 
> nit: typo
Done


PS2, Line 25: 
> nit: please make the string <= 72 characters long
Done


http://gerrit.cloudera.org:8080/#/c/7662/3/src/kudu/security/openssl_util.cc
File src/kudu/security/openssl_util.cc:

Line 149:   auto cleanup = MakeScopedCleanup([&]() {
> in the case of a bad PEM file format, I think this would end up leaving a p
Great point. I removed SCOPED_OPENSSL_NO_PENDING_ERRORS here and below.


Line 156:   // Iterate through the chain certificate and add each one to the 
stack.
> similar to above, I think this will result in returning null but with no pe
Agreed. I moved this to the calls Cert::FromString() and Cert::FromFile().

A test for this already exists in CertTest.CertInvalidInput, which works even 
after moving this from here.


PS3, Line 165: 
> the free call is no longer below. maybe just say 'the ScopedCleanup'
Done


Line 175:     if (ret <= 0) return ret;
> same
Done


Line 188:   }
> same
Done


Line 202: 
> same
Done


Line 212:   }
> same
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to