Alexey Serbin has posted comments on this change. Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs ......................................................................
Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/6514/11/src/kudu/rpc/negotiation-test.cc File src/kudu/rpc/negotiation-test.cc: PS11, Line 155: 192.169.0.0 This looks like an IP network address, not a single IP. Also, why not to just do return cur_addr->ParseString(...); Is it all intentional? If yes, please add a comment explaining those points. PS11, Line 222: server_socket = desc.use_test_socket ? unique_ptr<Socket>(new NegotiationTestSocket()) : : unique_ptr<Socket>(new Socket()); nit: why not to do that at the point of declaration of server_socket? http://gerrit.cloudera.org:8080/#/c/6514/11/src/kudu/rpc/server_negotiation.h File src/kudu/rpc/server_negotiation.h: PS11, Line 208: Sockaddr Is this signature intended for the move semantics? I'm not sure Sockaddr is a type which fits for the move semantics: when moving, it would be necessary to copy over the aggregated sockaddr_in structure field-by-field. Maybe it's better to have const reference instead? -- To view, visit http://gerrit.cloudera.org:8080/6514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Harsh J <ha...@harshj.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
