This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new d1a5488cd5 Review "Report malformed structure in PEM files as an error"
d1a5488cd5 is described below
commit d1a5488cd54fa294537502a6df164f80e6f90ca6
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Nov 3 21:21:31 2026 +0000
Review "Report malformed structure in PEM files as an error"
Fix a resource leak
Co-authored by: Codex / GPT-6.1-Sol
---
java/org/apache/tomcat/util/net/jsse/PEMFile.java | 18 +++---
.../apache/tomcat/util/net/jsse/TestPEMFile.java | 64 +++++++++++++++++++---
2 files changed, 66 insertions(+), 16 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/jsse/PEMFile.java
b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
index 0d6149090a..dd50662193 100644
--- a/java/org/apache/tomcat/util/net/jsse/PEMFile.java
+++ b/java/org/apache/tomcat/util/net/jsse/PEMFile.java
@@ -246,7 +246,11 @@ public class PEMFile {
public PEMFile(String filename, InputStream fileStream, String password,
String passwordFilename,
InputStream passwordFileStream, String keyAlgorithm) throws
IOException, GeneralSecurityException {
List<Part> parts = new ArrayList<>();
- try (BufferedReader reader = new BufferedReader(new
InputStreamReader(fileStream, StandardCharsets.US_ASCII))) {
+ String passwordToUse;
+ // Both streams are already open. Close the password stream even if
scanning the PEM data fails.
+ try (BufferedReader reader = new BufferedReader(new
InputStreamReader(fileStream, StandardCharsets.US_ASCII));
+ BufferedReader passwordReader = passwordFileStream == null ?
null :
+ new BufferedReader(new
InputStreamReader(passwordFileStream, StandardCharsets.UTF_8))) {
Part part = null;
String line;
while ((line = reader.readLine()) != null) {
@@ -291,16 +295,12 @@ public class PEMFile {
if (part != null) {
throw new IOException(sm.getString("pemFile.startWithoutEnd",
filename, part.type));
}
- }
- String passwordToUse;
- if (passwordFileStream != null) {
- try (BufferedReader reader =
- new BufferedReader(new
InputStreamReader(passwordFileStream, StandardCharsets.UTF_8))) {
- passwordToUse = reader.readLine();
+ if (passwordReader != null) {
+ passwordToUse = passwordReader.readLine();
+ } else {
+ passwordToUse = password;
}
- } else {
- passwordToUse = password;
}
for (Part part : parts) {
diff --git a/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java
b/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java
index 9a420b2e45..e706171dd2 100644
--- a/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java
+++ b/test/org/apache/tomcat/util/net/jsse/TestPEMFile.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.net.jsse;
import java.io.ByteArrayInputStream;
import java.io.File;
+import java.io.FilterInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
@@ -256,18 +257,67 @@ public class TestPEMFile {
}
+ @Test
+ public void testStreamsClosedAfterSuccessfulParse() throws Exception {
+ TrackingInputStream pemStream =
+ new TrackingInputStream(Files.readAllBytes(new
File(getPath(CERTIFICATE)).toPath()));
+ TrackingInputStream passwordStream = new
TrackingInputStream(KEY_PASSWORD.getBytes(StandardCharsets.UTF_8));
+
+ PEMFile pemFile = new PEMFile("certificate", pemStream, null,
"password", passwordStream, null);
+
+ Assert.assertEquals(1, pemFile.getCertificates().size());
+ Assert.assertTrue(pemStream.closed);
+ Assert.assertTrue(passwordStream.closed);
+ }
+
+
+ @Test
+ public void testStreamsClosedAfterPemReadError() throws Exception {
+ TrackingInputStream pemStream = new TrackingInputStream(new byte[0]) {
+ @Override
+ public int read(byte[] bytes, int offset, int length) throws
IOException {
+ throw new IOException("PEM read failed");
+ }
+ };
+ TrackingInputStream passwordStream = new
TrackingInputStream(KEY_PASSWORD.getBytes(StandardCharsets.UTF_8));
+
+ IOException exception = Assert.assertThrows(IOException.class,
+ () -> new PEMFile("unreadable", pemStream, null, "password",
passwordStream, null));
+
+ Assert.assertEquals("PEM read failed", exception.getMessage());
+ Assert.assertTrue(pemStream.closed);
+ Assert.assertTrue(passwordStream.closed);
+ }
+
+
/*
* The structure of the file is checked while it is scanned, before any of
the content is decoded, so these
* malformed structures must be rejected even though the content is not
valid PEM encoded data.
*/
private void assertInvalidPem(String content) throws Exception {
- ByteArrayInputStream stream = new
ByteArrayInputStream(content.getBytes(StandardCharsets.US_ASCII));
- try {
- @SuppressWarnings("unused")
- PEMFile pf = new PEMFile("invalid", stream, null, null);
- Assert.fail("Expected the malformed PEM to be rejected");
- } catch (IOException ioe) {
- // Expected
+ TrackingInputStream pemStream = new
TrackingInputStream(content.getBytes(StandardCharsets.US_ASCII));
+ TrackingInputStream passwordStream = new
TrackingInputStream(KEY_PASSWORD.getBytes(StandardCharsets.UTF_8));
+
+ Assert.assertThrows(IOException.class,
+ () -> new PEMFile("invalid", pemStream, null, "password",
passwordStream, null));
+
+ // Both streams are opened before scanning, so structural errors must
close the password stream too.
+ Assert.assertTrue(pemStream.closed);
+ Assert.assertTrue(passwordStream.closed);
+ }
+
+
+ private static class TrackingInputStream extends FilterInputStream {
+ private boolean closed;
+
+ TrackingInputStream(byte[] content) {
+ super(new ByteArrayInputStream(content));
+ }
+
+ @Override
+ public void close() throws IOException {
+ closed = true;
+ super.close();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
