gaurav singh 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 10: (3 comments) 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? As per the RFC document, they are not optional when x5c is provided. http://gerrit.cloudera.org:8080/#/c/21382/8/be/src/util/jwt-util.cc@381 PS8, Line 381: O_puts(bio, data); > add { } around return statement. good point. The man page for PEM_read_bio_X509() doesn't mention whether it frees it not and also no examples in openssl docs. I think its good to free it if PEM_read_bio_X509 errors out. http://gerrit.cloudera.org:8080/#/c/21382/8/be/src/util/jwt-util.cc@407 PS8, Line 407: pu > nit: extra indent spaces. Done -- 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: 10 Gerrit-Owner: gaurav singh <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: gaurav singh <[email protected]> Gerrit-Comment-Date: Wed, 08 May 2024 15:11:02 +0000 Gerrit-HasComments: Yes
