Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21382 )
Change subject: IMPALA-12559: Support x5c Parameter for RSA JSON Web Keys ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/21382/7/be/src/util/jwt-util.cc File be/src/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/21382/7/be/src/util/jwt-util.cc@320 PS7, Line 320: : Status status; : JWTPublicKey* jwt_pub_key = nullptr; : try { : if (algorithm == "hs256") { : jwt_pub_key = new HS256JWTPublicKey(algorithm, it_k->second); : } else if (algorithm == "hs What's the reason to remove x5c? http://gerrit.cloudera.org:8080/#/c/21382/8/be/src/util/jwt-util.cc File be/src/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/21382/8/be/src/util/jwt-util.cc@356 PS8, Line 356: if (it_n == kv_map.end() || it_e == kv_map.end()) { : return Status("'n' and 'e' properties are required"); : } else if (it_n->second.empty() || it_e->second.empty()) { : return Status("'n' and 'e' properties must be a non-empty string"); : } Are "n" and "e" optional when 'x5c' is provided? http://gerrit.cloudera.org:8080/#/c/21382/8/be/src/util/jwt-util.cc@381 PS8, Line 381: return Status(Substitute("Invalid x5c certificate")); add { } around return statement. Should we free bio before return? http://gerrit.cloudera.org:8080/#/c/21382/8/be/src/util/jwt-util.cc@407 PS8, Line 407: nit: extra indent spaces. -- To view, visit http://gerrit.cloudera.org:8080/21382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I70be6f9f54190544aa005b2644e2ed8db6f6bb74 Gerrit-Change-Number: 21382 Gerrit-PatchSet: 8 Gerrit-Owner: gaurav singh <gsi...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: gaurav singh <gsi...@cloudera.com> Gerrit-Comment-Date: Wed, 08 May 2024 00:12:45 +0000 Gerrit-HasComments: Yes
