----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49400/#review140164 -----------------------------------------------------------
Fix it, then Ship it! 3rdparty/libprocess/include/process/ssl/utilities.hpp (line 61) <https://reviews.apache.org/r/49400/#comment205465> how about: "... will use the ip for a subject alternative name `iPAddress` extension." 3rdparty/libprocess/include/process/ssl/utilities.hpp (line 73) <https://reviews.apache.org/r/49400/#comment205466> mention the `iPAdrress` extension as per your higher comment 3rdparty/libprocess/src/ssl/utilities.cpp (line 217) <https://reviews.apache.org/r/49400/#comment205468> Here you say `create`, below you say `malloc` 3rdparty/libprocess/src/ssl/utilities.cpp (line 234) <https://reviews.apache.org/r/49400/#comment205467> Capitalize `Failed` 3rdparty/libprocess/src/ssl/utilities.cpp (line 239) <https://reviews.apache.org/r/49400/#comment205469> Why should we abort it the family type was unsupported (`ip.get().in()` returns an error). Our function returns a `Try`, can we propagate the error? 3rdparty/libprocess/src/ssl/utilities.cpp (lines 240 - 243) <https://reviews.apache.org/r/49400/#comment205470> I'm not sure I understand why this works. `in_addr.get().s_addr` is a uint. Aren't we supposed to be copying a string in this case? If it *is* supposed to be a binary IP this definitely deserves a comment. The documentation doesn't make it clear to me that this can be binary instead of a string. 3rdparty/libprocess/src/ssl/utilities.cpp (line 247) <https://reviews.apache.org/r/49400/#comment205471> Capitalize `Failed` 3rdparty/libprocess/src/ssl/utilities.cpp (line 252) <https://reviews.apache.org/r/49400/#comment205475> If this transfers ownership of the memory for `alt_name` to the `alt_name_stack` can we add a comment here. It's easier to follow the memory cleanup below if we know why we're not cleaning up the `alt_name` by itself anymore. 3rdparty/libprocess/src/ssl/utilities.cpp (line 256) <https://reviews.apache.org/r/49400/#comment205472> Capitalize `Failed` 3rdparty/libprocess/src/ssl/utilities.cpp (lines 266 - 267) <https://reviews.apache.org/r/49400/#comment205476> Inconsistent use of `alt_name` in the error messages 3rdparty/libprocess/src/ssl/utilities.cpp (line 267) <https://reviews.apache.org/r/49400/#comment205473> Capitalize `Failed`. - Joris Van Remoortere On June 30, 2016, 12:19 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49400/ > ----------------------------------------------------------- > > (Updated June 30, 2016, 12:19 a.m.) > > > Review request for mesos, Adam B, Albert Strasheim, Artem Harutyunyan, Joris > Van Remoortere, and Lukas Loesche. > > > Bugs: MESOS-5724 > https://issues.apache.org/jira/browse/MESOS-5724 > > > Repository: mesos > > > Description > ------- > > Adds the ability to render a subject alternative name based on a given > IP address within a X509 certificate extension. Additionally the > libprocess test suite makes use of this feature. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/ssl/gtest.hpp 5435ddd > 3rdparty/libprocess/include/process/ssl/utilities.hpp ad9ec5d > 3rdparty/libprocess/src/ssl/utilities.cpp d23f462 > > Diff: https://reviews.apache.org/r/49400/diff/ > > > Testing > ------- > > make check on OSX and various linux distros. > > Functional testing by validating a rendered certificate; > > ``` > openssl x509 -text -noout -in "temp_cert_file_name" > ``` > > > Thanks, > > Till Toenshoff > >
