Skip to content

Commit 466b631

Browse files
Fix resource leaks in SamlSettingsHolder, SamlValidator, and IndexResource
- SamlSettingsHolder: Wrap FileInputStream in try-with-resources in initDefaultSettings() to prevent file descriptor leak - SamlValidator: Add try-finally with conn.disconnect() in validateIdpConnectivity() to prevent TCP socket leak; close InputStream in readResponseSnippet(); close DeflaterOutputStream and Deflater in createTestSamlRequest() - IndexResource: Wrap InputStream/BufferedReader in try-with-resources in both constructor and getIndexFile(); add null check for missing classpath resource
1 parent 9da7bea commit 466b631

File tree

3 files changed

+82
-60
lines changed

3 files changed

+82
-60
lines changed

openmetadata-service/src/main/java/org/openmetadata/service/resources/system/IndexResource.java

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,17 @@ public class IndexResource {
1818
private String indexHtml;
1919

2020
public IndexResource() {
21-
InputStream inputStream = getClass().getResourceAsStream("/assets/index.html");
22-
indexHtml =
23-
new BufferedReader(new InputStreamReader(inputStream))
24-
.lines()
25-
.collect(Collectors.joining("\n"));
21+
try (InputStream inputStream = getClass().getResourceAsStream("/assets/index.html")) {
22+
if (inputStream == null) {
23+
throw new IllegalStateException("Resource /assets/index.html not found on classpath");
24+
}
25+
indexHtml =
26+
new BufferedReader(new InputStreamReader(inputStream))
27+
.lines()
28+
.collect(Collectors.joining("\n"));
29+
} catch (java.io.IOException e) {
30+
throw new IllegalStateException("Failed to read /assets/index.html", e);
31+
}
2632
}
2733

2834
public void initialize(OpenMetadataApplicationConfig config) {
@@ -32,11 +38,18 @@ public void initialize(OpenMetadataApplicationConfig config) {
3238
public static String getIndexFile(String basePath) {
3339
LOG.info("IndexResource.getIndexFile called with basePath: [{}]", basePath);
3440

35-
InputStream inputStream = IndexResource.class.getResourceAsStream("/assets/index.html");
36-
String indexHtml =
37-
new BufferedReader(new InputStreamReader(inputStream))
38-
.lines()
39-
.collect(Collectors.joining("\n"));
41+
String indexHtml;
42+
try (InputStream inputStream = IndexResource.class.getResourceAsStream("/assets/index.html")) {
43+
if (inputStream == null) {
44+
throw new IllegalStateException("Resource /assets/index.html not found on classpath");
45+
}
46+
indexHtml =
47+
new BufferedReader(new InputStreamReader(inputStream))
48+
.lines()
49+
.collect(Collectors.joining("\n"));
50+
} catch (java.io.IOException e) {
51+
throw new IllegalStateException("Failed to read /assets/index.html", e);
52+
}
4053

4154
String result = indexHtml.replace("${basePath}", basePath);
4255
String basePathLine =

openmetadata-service/src/main/java/org/openmetadata/service/security/auth/validator/SamlValidator.java

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -294,46 +294,50 @@ private FieldError validateIdpConnectivity(SamlSSOClientConfig samlConfig) {
294294

295295
URL url = new URL(urlWithParams);
296296
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
297-
conn.setRequestMethod("GET");
298-
conn.setConnectTimeout(5000);
299-
conn.setReadTimeout(5000);
300-
conn.setInstanceFollowRedirects(false);
301-
int responseCode = conn.getResponseCode();
302-
LOG.debug("IdP response code to SAML request: {}", responseCode);
303-
304-
// Analyze response
305-
if (responseCode == 404) {
306-
return ValidationErrorBuilder.createFieldError(
307-
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
308-
"SSO Login URL not found (HTTP 404). The URL '" + ssoUrl + "' does not exist.");
309-
} else if (responseCode == 405) {
310-
return ValidationErrorBuilder.createFieldError(
311-
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
312-
"SSO URL doesn't accept GET requests (HTTP 405). Please check the SSO URL configuration.");
313-
} else if (responseCode >= 200 && responseCode < 400) {
314-
// 200 or 302 means the IdP accepted our SAML request
315-
return null; // Success - SSO Login URL validated
316-
} else if (responseCode >= 400 && responseCode < 500) {
317-
// 400-499 could mean wrong URL or IdP rejecting the request
318-
// Read a bit of the response to check for specific errors
319-
String responseSnippet = readResponseSnippet(conn);
320-
if (responseSnippet.toLowerCase().contains("saml")
321-
|| responseSnippet.toLowerCase().contains("invalid")) {
322-
// Warning case - treat as success
323-
LOG.warn(
324-
"SSO URL responded with client error (HTTP {}). This might be due to the test SAML request format.",
325-
responseCode);
326-
return null;
297+
try {
298+
conn.setRequestMethod("GET");
299+
conn.setConnectTimeout(5000);
300+
conn.setReadTimeout(5000);
301+
conn.setInstanceFollowRedirects(false);
302+
int responseCode = conn.getResponseCode();
303+
LOG.debug("IdP response code to SAML request: {}", responseCode);
304+
305+
// Analyze response
306+
if (responseCode == 404) {
307+
return ValidationErrorBuilder.createFieldError(
308+
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
309+
"SSO Login URL not found (HTTP 404). The URL '" + ssoUrl + "' does not exist.");
310+
} else if (responseCode == 405) {
311+
return ValidationErrorBuilder.createFieldError(
312+
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
313+
"SSO URL doesn't accept GET requests (HTTP 405). Please check the SSO URL configuration.");
314+
} else if (responseCode >= 200 && responseCode < 400) {
315+
// 200 or 302 means the IdP accepted our SAML request
316+
return null; // Success - SSO Login URL validated
317+
} else if (responseCode >= 400 && responseCode < 500) {
318+
// 400-499 could mean wrong URL or IdP rejecting the request
319+
// Read a bit of the response to check for specific errors
320+
String responseSnippet = readResponseSnippet(conn);
321+
if (responseSnippet.toLowerCase().contains("saml")
322+
|| responseSnippet.toLowerCase().contains("invalid")) {
323+
// Warning case - treat as success
324+
LOG.warn(
325+
"SSO URL responded with client error (HTTP {}). This might be due to the test SAML request format.",
326+
responseCode);
327+
return null;
328+
}
329+
return ValidationErrorBuilder.createFieldError(
330+
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
331+
"SSO URL returned error (HTTP "
332+
+ responseCode
333+
+ "). Please verify the URL is correct.");
334+
} else {
335+
return ValidationErrorBuilder.createFieldError(
336+
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
337+
"SSO URL is not accessible (HTTP " + responseCode + ")");
327338
}
328-
return ValidationErrorBuilder.createFieldError(
329-
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
330-
"SSO URL returned error (HTTP "
331-
+ responseCode
332-
+ "). Please verify the URL is correct.");
333-
} else {
334-
return ValidationErrorBuilder.createFieldError(
335-
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
336-
"SSO URL is not accessible (HTTP " + responseCode + ")");
339+
} finally {
340+
conn.disconnect();
337341
}
338342
} catch (Exception e) {
339343
LOG.warn("SSO URL validation failed", e);
@@ -376,10 +380,13 @@ private String createTestSamlRequest(SamlSSOClientConfig samlConfig) {
376380
java.io.ByteArrayOutputStream bytesOut = new java.io.ByteArrayOutputStream();
377381
java.util.zip.Deflater deflater =
378382
new java.util.zip.Deflater(java.util.zip.Deflater.DEFLATED, true);
379-
java.util.zip.DeflaterOutputStream deflaterStream =
380-
new java.util.zip.DeflaterOutputStream(bytesOut, deflater);
381-
deflaterStream.write(samlRequestXml.getBytes(StandardCharsets.UTF_8));
382-
deflaterStream.finish();
383+
try (java.util.zip.DeflaterOutputStream deflaterStream =
384+
new java.util.zip.DeflaterOutputStream(bytesOut, deflater)) {
385+
deflaterStream.write(samlRequestXml.getBytes(StandardCharsets.UTF_8));
386+
deflaterStream.finish();
387+
} finally {
388+
deflater.end();
389+
}
383390

384391
// Base64 encode
385392
String base64Request = Base64.getEncoder().encodeToString(bytesOut.toByteArray());
@@ -400,10 +407,12 @@ private String readResponseSnippet(HttpURLConnection conn) {
400407
inputStream = conn.getInputStream();
401408
}
402409
if (inputStream != null) {
403-
byte[] buffer = new byte[500];
404-
int bytesRead = inputStream.read(buffer);
405-
if (bytesRead > 0) {
406-
return new String(buffer, 0, bytesRead, StandardCharsets.UTF_8);
410+
try (inputStream) {
411+
byte[] buffer = new byte[500];
412+
int bytesRead = inputStream.read(buffer);
413+
if (bytesRead > 0) {
414+
return new String(buffer, 0, bytesRead, StandardCharsets.UTF_8);
415+
}
407416
}
408417
}
409418
} catch (Exception e) {

openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ public void initDefaultSettings(OpenMetadataApplicationConfig catalogApplication
118118
&& !CommonUtil.nullOrEmpty(securityConfig.getKeyStorePassword())
119119
&& !CommonUtil.nullOrEmpty(securityConfig.getKeyStoreAlias())) {
120120
KeyStore keyStore = KeyStore.getInstance("JKS");
121-
keyStore.load(
122-
new FileInputStream(securityConfig.getKeyStoreFilePath()),
123-
securityConfig.getKeyStorePassword().toCharArray());
121+
try (FileInputStream fis = new FileInputStream(securityConfig.getKeyStoreFilePath())) {
122+
keyStore.load(fis, securityConfig.getKeyStorePassword().toCharArray());
123+
}
124124
samlData.put(SettingsBuilder.KEYSTORE_KEY, keyStore);
125125
samlData.put(SettingsBuilder.KEYSTORE_ALIAS, securityConfig.getKeyStoreAlias());
126126
samlData.put(SettingsBuilder.KEYSTORE_KEY_PASSWORD, securityConfig.getKeyStorePassword());

0 commit comments

Comments
 (0)