You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "rtadepalli (via GitHub)" <gi...@apache.org> on 2025/06/05 02:07:46 UTC
[PR] GH-155: Delegate opening mTLS cert files to `build()` [arrow-java]
rtadepalli opened a new pull request, #783:
URL: https://github.com/apache/arrow-java/pull/783
## What's Changed
Make `FlightServer.Builder` store `File` and delegate opening the file to the actual `build()` method.
Closes #155.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-155: Delegate opening mTLS cert files to `build()` [arrow-java]
Posted by "rtadepalli (via GitHub)" <gi...@apache.org>.
rtadepalli commented on PR #783:
URL: https://github.com/apache/arrow-java/pull/783#issuecomment-2968739682
Could I please get a review @lidavidm thanks.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-155: Delegate opening mTLS cert files to `build()` [arrow-java]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-155: Delegate opening mTLS cert files to `build()` [arrow-java]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #783:
URL: https://github.com/apache/arrow-java/pull/783#issuecomment-2968745275
Sorry - I have basically 0 time for arrow-java recently. I will try to get to all the reviews soon but I can't make any promises.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org