lidavidm commented on code in PR #783:
URL: https://github.com/apache/arrow-java/pull/783#discussion_r2160589399
##########
flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -552,5 +570,20 @@ public Builder producer(FlightProducer producer) {
this.producer = Preconditions.checkNotNull(producer);
return this;
}
+
+ private void prepareTlsSettings() throws IOException {
Review Comment:
The way this is written, if you set the File version then the deprecated
version never takes effect, even if you set it afterwards. I suppose that's OK
given the deprecation.
##########
flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java:
##########
@@ -213,6 +222,16 @@ public static final class Builder {
/** Create the server for this builder. */
public FlightServer build() {
+ // Get TLS info in order if the server is being configured to use mTLS.
+ try {
+ prepareTlsSettings();
+ } catch (IOException e) {
+ closeMTlsCACert();
+ closeCertChain();
+ closeKey();
Review Comment:
Honestly, this should be in a try-finally wrapping the entire build() call
(since other exceptions in between this and the actual TLS settings will leak
the handles). Either that, or this should be moved down to right before we use
the file handles (and should still be in a try-finally)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
