Fixes #27091 : Fix resource leaks in SamlSettingsHolder, SamlValidator, and IndexResource

[RajdeepKushwaha5]

Fix resource leaks in SamlSettingsHolder, SamlValidator, and IndexResource

Fixes #27091

Describe your changes

Close three resource leak bugs across openmetadata-service where streams and connections were opened but never closed, leaking file descriptors and TCP sockets.

SamlSettingsHolder — FileInputStream never closed
SamlSettingsHolder.initDefaultSettings() created a FileInputStream inline inside keyStore.load(new FileInputStream(...)). Since the stream reference was never assigned to a variable, it could never be closed — leaking a file descriptor on every SAML settings load.

Fix: Wrap in try-with-resources:

try (FileInputStream fis = new FileInputStream(securityConfig.getKeyStoreFilePath())) {
    keyStore.load(fis, securityConfig.getKeyStorePassword().toCharArray());
}

SamlValidator — HttpURLConnection never disconnected
SamlValidator.validateIdpConnectivity() opened an HttpURLConnection to test the IdP SSO URL but never called conn.disconnect() in any code path, leaking a TCP socket per validation. Additionally:

• readResponseSnippet() read from the connection's error/input stream without closing it
• createTestSamlRequest() created a DeflaterOutputStream and Deflater without closing them

Fix: Wrapped the connection usage in try-finally with conn.disconnect(), wrapped readResponseSnippet's stream in try-with-resources, and wrapped the deflater streams in try-with-resources with deflater.end() in finally.

IndexResource — InputStream and BufferedReader never closed
Both the constructor and getIndexFile() opened an InputStream via getResourceAsStream() and wrapped it in a BufferedReader, but neither was ever closed. Additionally, no null check existed — a missing resource would cause an unhelpful NPE.

Fix: Wrapped both methods in try-with-resources with null checks that throw IllegalStateException with clear context.

This continues the resource leak cleanup pattern from PR #27063, which fixed identical InputStream leaks in SchemaFieldExtractor.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read the CONTRIBUTING document
• I have performed a self-review of my own code
• My code follows the style guidelines of this project (mvn spotless:check passes)
• I have commented my code, particularly in hard-to-understand areas
• New and existing unit tests pass locally with my changes

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[Copilot]

Pull request overview

This PR addresses resource-leak issues in the backend (openmetadata-service) by ensuring streams and network connections are properly closed in SAML-related code and the root index HTML resource handler.

Changes:

• Use try-with-resources to close the keystore FileInputStream in SamlSettingsHolder.
• Ensure HttpURLConnection is always disconnected and related streams/deflaters are closed in SamlValidator.
• Close classpath resource streams in IndexResource and add explicit “resource missing” errors instead of NPEs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java	Wraps keystore file loading in try-with-resources to avoid FD leaks.
openmetadata-service/src/main/java/org/openmetadata/service/security/auth/validator/SamlValidator.java	Adds connection disconnect + stream/deflater lifecycle management during IdP connectivity validation and request generation.
openmetadata-service/src/main/java/org/openmetadata/service/resources/system/IndexResource.java	Closes /assets/index.html classpath stream and throws clear errors when missing/unreadable.

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Fixes resource leaks in SamlSettingsHolder, SamlValidator, and IndexResource by adding proper try-with-resources blocks and ensuring BufferedReader cleanup. All issues resolved with no remaining findings.

✅ 2 resolved

✅ Bug: try (inputStream) won't compile: variable is not effectively final

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/auth/validator/SamlValidator.java:405-410
In readResponseSnippet(), the variable inputStream is declared and assigned on line 405 (conn.getErrorStream()), then conditionally reassigned on line 407 (conn.getInputStream()). Java 9+ try (existingVar) syntax requires the variable to be effectively final (assigned exactly once). Since inputStream can be assigned twice, the compiler will reject try (inputStream) on line 410 with an error like: "Variable used as a try-with-resources resource is not final or effectively final".

This PR will not compile.

✅ Quality: BufferedReader not closed in IndexResource try-with-resources

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/system/IndexResource.java:21-28 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/system/IndexResource.java:42-49
In both the constructor and getIndexFile(), a BufferedReader wrapping an InputStreamReader is created inline but only the underlying InputStream is managed by try-with-resources. While this doesn't cause data loss (since we're only reading), it's cleaner to include the BufferedReader in the try-with-resources to properly release its internal buffer and follow the pattern consistently.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.