Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18468 )
Change subject: jwt: add test for fetching JWKS via URL ...................................................................... Patch Set 13: (2 comments) http://gerrit.cloudera.org:8080/#/c/18468/13/src/kudu/util/jwt-util-test.cc File src/kudu/util/jwt-util-test.cc: http://gerrit.cloudera.org:8080/#/c/18468/13/src/kudu/util/jwt-util-test.cc@1194 PS13, Line 1194: Sockaddr addr_; nit: it seems this is unused, as pointed by Wenzhe in one of earlier revisions http://gerrit.cloudera.org:8080/#/c/18468/13/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/18468/13/src/kudu/util/jwt-util.cc@202 PS13, Line 202: RETURN_NOT_OK(HSJWTPublicKeyBuilder::CreateJWKPublicKey(kv_map, &jwt_pub_key)); : jwks_->AddHSKey(key_id, jwt_pub_key); I took a closer look at the code, and I'm not sure I understand what's are the rules of ownership for the new keys allocated by various CreateJWKPublicKey() methods. One issue I see that those raw pointers are put into corresponding maps by jwks_->AddHSKey() and methods alike. However, if the key is already there, nothing is done (except for logging a warning) and the 'jwt_pub_key' is then leaked. With that, does it make sense to change the signature of CreateJWKPublicKey() methods and AddXxxPublicKey() methods to operate with unique pointers instead of raw pointers to avoid memory leaks in such situations? -- To view, visit http://gerrit.cloudera.org:8080/18468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ief272c813b62e789d747a88e1f3be8c406eed3f8 Gerrit-Change-Number: 18468 Gerrit-PatchSet: 13 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Fri, 11 Nov 2022 21:21:03 +0000 Gerrit-HasComments: Yes
