Skip to content

Commit cf03391

Browse files
dixitdeepakdixitdeepak
committed
Use try-with-resources for owned resources (OFBIZ-13399)
Replace manual resource closing with try-with-resources in utility code. Scope certificate, object serialization, XML URL streams, and sequence JDBC resources to their owning blocks. Keep caller-owned XML input streams open in UtilProperties.
1 parent 4173bd0 commit cf03391

5 files changed

Lines changed: 94 additions & 115 deletions

File tree

‎framework/base/src/main/java/org/apache/ofbiz/base/util/KeyStoreUtil.java‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,9 @@ public static Certificate pemToCert(String certString) throws IOException, Certi
188188
}
189189

190190
public static Certificate pemToCert(File certFile) throws IOException, CertificateException {
191-
return pemToCert(new FileInputStream(certFile));
191+
try (InputStream is = new FileInputStream(certFile)) {
192+
return pemToCert(is);
193+
}
192194
}
193195

194196
public static Certificate pemToCert(InputStream is) throws IOException, CertificateException {
@@ -201,31 +203,30 @@ public static Certificate pemToCert(Reader r) throws IOException, CertificateExc
201203

202204
BufferedReader reader = new BufferedReader(r);
203205
ByteArrayOutputStream baos = new ByteArrayOutputStream();
204-
PrintStream ps = new PrintStream(baos, false, StandardCharsets.UTF_8.toString());
205-
206-
String line;
206+
try (PrintStream ps = new PrintStream(baos, false, StandardCharsets.UTF_8.toString())) {
207+
String line;
207208

208-
// ignore up to the header
209-
while ((line = reader.readLine()) != null && !line.equals(header)) {
210-
Debug.logVerbose("Ignore up to the header...", MODULE);
211-
}
209+
// ignore up to the header
210+
while ((line = reader.readLine()) != null && !line.equals(header)) {
211+
Debug.logVerbose("Ignore up to the header...", MODULE);
212+
}
212213

213-
// no header found
214-
if (line == null) {
215-
throw new IOException("Error reading certificate, missing BEGIN boundary");
216-
}
214+
// no header found
215+
if (line == null) {
216+
throw new IOException("Error reading certificate, missing BEGIN boundary");
217+
}
217218

218-
// in between the header and footer is the actual certificate
219-
while ((line = reader.readLine()) != null && !line.equals(footer)) {
220-
line = line.replaceAll("\\s", "");
221-
ps.print(line);
222-
}
219+
// in between the header and footer is the actual certificate
220+
while ((line = reader.readLine()) != null && !line.equals(footer)) {
221+
line = line.replaceAll("\\s", "");
222+
ps.print(line);
223+
}
223224

224-
// no footer found
225-
if (line == null) {
226-
throw new IOException("Error reading certificate, missing END boundary");
225+
// no footer found
226+
if (line == null) {
227+
throw new IOException("Error reading certificate, missing END boundary");
228+
}
227229
}
228-
ps.close();
229230

230231
// decode the buffer to a X509Certificate
231232

‎framework/base/src/main/java/org/apache/ofbiz/base/util/UtilObject.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,11 @@ public static byte[] getBytes(Object obj) {
6464
* @throws IOException
6565
*/
6666
public static long getByteCount(Object obj) throws IOException {
67-
ByteArrayOutputStream bos = new ByteArrayOutputStream();
68-
ObjectOutputStream oos = new ObjectOutputStream(bos);
69-
oos.writeObject(obj);
70-
oos.flush();
71-
long size = bos.size();
72-
bos.close();
73-
return size;
67+
try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); ObjectOutputStream oos = new ObjectOutputStream(bos)) {
68+
oos.writeObject(obj);
69+
oos.flush();
70+
return bos.size();
71+
}
7472
}
7573

7674
/** Deserialize a byte array back to an object; if there is an error, it is logged, and null is returned. */

‎framework/base/src/main/java/org/apache/ofbiz/base/util/UtilProperties.java‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -303,26 +303,18 @@ public static String getPropertyValue(String resource, String name) {
303303
*/
304304
public static Properties createProperties(String fileName) {
305305
Assert.notEmpty("fileName", fileName);
306-
InputStream inStream = null;
307306
try {
308307
URL url = Thread.currentThread().getContextClassLoader().getResource(fileName);
309308
if (url == null) {
310309
return null;
311310
}
312-
inStream = url.openStream();
313-
Properties properties = new Properties();
314-
properties.load(inStream);
315-
return properties;
311+
try (InputStream inStream = url.openStream()) {
312+
Properties properties = new Properties();
313+
properties.load(inStream);
314+
return properties;
315+
}
316316
} catch (Exception e) {
317317
throw new IllegalStateException("Exception thrown while reading " + fileName + ": " + e);
318-
} finally {
319-
if (inStream != null) {
320-
try {
321-
inStream.close();
322-
} catch (IOException e) {
323-
Debug.logError(e, "Exception thrown while closing InputStream", MODULE);
324-
}
325-
}
326318
}
327319
}
328320

@@ -963,10 +955,8 @@ public static Properties xmlToProperties(InputStream in, Locale locale, Properti
963955
Document doc = null;
964956
try {
965957
doc = UtilXml.readXmlDocument(in, true, "XML Properties file");
966-
in.close();
967958
} catch (Exception e) {
968959
Debug.logWarning(e, "XML file for locale " + locale + " could not be loaded.", MODULE);
969-
in.close();
970960
return null;
971961
}
972962
Element resourceElement = doc.getDocumentElement();
@@ -1138,11 +1128,7 @@ public ExtendedProperties(Properties defaults) {
11381128
}
11391129
@Override
11401130
public synchronized void loadFromXML(InputStream in) throws IOException, InvalidPropertiesFormatException {
1141-
try {
1142-
xmlToProperties(in, null, this);
1143-
} finally {
1144-
in.close();
1145-
}
1131+
xmlToProperties(in, null, this);
11461132
}
11471133
}
11481134
}

‎framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,10 +429,9 @@ public static Document readXmlDocument(URL url, boolean validate, boolean withPo
429429
throw new IOException("Domain " + urlHost + " not accepted to prevent host header injection."
430430
+ " You need to set host-headers-allowed property in security.properties file.");
431431
}
432-
InputStream is = url.openStream();
433-
Document document = readXmlDocument(is, validate, url.toString(), withPosition);
434-
is.close();
435-
return document;
432+
try (InputStream is = url.openStream()) {
433+
return readXmlDocument(is, validate, url.toString(), withPosition);
434+
}
436435
}
437436

438437
public static Document readXmlDocument(InputStream is, String docDescription)

‎framework/entity/src/main/java/org/apache/ofbiz/entity/util/SequenceUtil.java‎

Lines changed: 58 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -201,76 +201,72 @@ private void fillBank(long stagger) {
201201
try {
202202
beganTransaction = TransactionUtil.begin();
203203

204-
Connection connection = null;
205-
Statement stmt = null;
206-
ResultSet rs = null;
207-
208-
try {
209-
connection = TransactionFactoryLoader.getInstance().getConnection(SequenceUtil.this.helperInfo);
210-
} catch (SQLException sqle) {
211-
Debug.logWarning("Unable to establish a connection with the database. Error was:" + sqle.toString(), MODULE);
212-
throw sqle;
213-
} catch (GenericEntityException e) {
214-
Debug.logWarning("Unable to establish a connection with the database. Error was: " + e.toString(), MODULE);
215-
throw e;
216-
}
217-
if (connection == null) {
218-
throw new GenericEntityException("Unable to establish a connection with the database, connection was null...");
219-
}
204+
boolean committed = false;
205+
boolean connectionAvailable = false;
206+
try (Connection connection = TransactionFactoryLoader.getInstance().getConnection(SequenceUtil.this.helperInfo)) {
207+
if (connection == null) {
208+
throw new GenericEntityException("Unable to establish a connection with the database, connection was null...");
209+
}
210+
connectionAvailable = true;
211+
212+
try (Statement stmt = connection.createStatement()) {
213+
String sql = null;
214+
// 1 - run an update with no changes to get a lock on the record
215+
if (stmt.executeUpdate(updateForLockStatement) <= 0) {
216+
Debug.logWarning("Lock failed; no sequence row was found, will try to add a new one for sequence: " + seqName,
217+
MODULE);
218+
sql = "INSERT INTO " + SequenceUtil.this.tableName + " (" + SequenceUtil.this.nameColName + ", "
219+
+ SequenceUtil.this.idColName + ") VALUES ('" + this.seqName + "', " + START_SEQ_ID + ")";
220+
try {
221+
stmt.executeUpdate(sql);
222+
} catch (SQLException sqle) {
223+
// insert failed: this means that another thread inserted the record;
224+
// then retry to run an update with no changes to
225+
// get a lock on the record
226+
if (stmt.executeUpdate(updateForLockStatement) <= 0) {
227+
// This should never happen
228+
throw new GenericEntityException("No rows changed when trying insert new sequence: " + seqName);
229+
}
220230

221-
try {
222-
stmt = connection.createStatement();
223-
String sql = null;
224-
// 1 - run an update with no changes to get a lock on the record
225-
if (stmt.executeUpdate(updateForLockStatement) <= 0) {
226-
Debug.logWarning("Lock failed; no sequence row was found, will try to add a new one for sequence: " + seqName, MODULE);
227-
sql = "INSERT INTO " + SequenceUtil.this.tableName + " (" + SequenceUtil.this.nameColName + ", "
228-
+ SequenceUtil.this.idColName + ") VALUES ('" + this.seqName + "', " + START_SEQ_ID + ")";
229-
try {
230-
stmt.executeUpdate(sql);
231-
} catch (SQLException sqle) {
232-
// insert failed: this means that another thread inserted the record; then retry to run an update with no changes to
233-
// get a lock on the record
234-
if (stmt.executeUpdate(updateForLockStatement) <= 0) {
235-
// This should never happen
236-
throw new GenericEntityException("No rows changed when trying insert new sequence: " + seqName);
237231
}
238-
239232
}
240-
}
241-
// 2 - select the record (now locked) to get the curSeqId
242-
rs = stmt.executeQuery(selectSequenceStatement);
243-
boolean sequenceFound = rs.next();
244-
if (sequenceFound) {
245-
curSeqId = rs.getLong(SequenceUtil.this.idColName);
246-
}
247-
rs.close();
248-
if (!sequenceFound) {
249-
throw new GenericEntityException("Failed to find the sequence record for sequence: " + seqName);
250-
}
251-
// 3 - increment the sequence
252-
sql = "UPDATE " + SequenceUtil.this.tableName + " SET " + SequenceUtil.this.idColName + "=" + SequenceUtil.this.idColName
253-
+ "+" + bankSize + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
254-
if (stmt.executeUpdate(sql) <= 0) {
255-
throw new GenericEntityException("Update failed, no rows changes for seqName: " + seqName);
256-
}
233+
// 2 - select the record (now locked) to get the curSeqId
234+
boolean sequenceFound;
235+
try (ResultSet rs = stmt.executeQuery(selectSequenceStatement)) {
236+
sequenceFound = rs.next();
237+
if (sequenceFound) {
238+
curSeqId = rs.getLong(SequenceUtil.this.idColName);
239+
}
240+
}
241+
if (!sequenceFound) {
242+
throw new GenericEntityException("Failed to find the sequence record for sequence: " + seqName);
243+
}
244+
// 3 - increment the sequence
245+
sql = "UPDATE " + SequenceUtil.this.tableName + " SET " + SequenceUtil.this.idColName + "=" + SequenceUtil.this.idColName
246+
+ "+" + bankSize + " WHERE " + SequenceUtil.this.nameColName + "='" + this.seqName + "'";
247+
if (stmt.executeUpdate(sql) <= 0) {
248+
throw new GenericEntityException("Update failed, no rows changes for seqName: " + seqName);
249+
}
257250

258-
TransactionUtil.commit(beganTransaction);
251+
TransactionUtil.commit(beganTransaction);
252+
committed = true;
253+
}
259254

260255
} catch (SQLException sqle) {
261-
Debug.logWarning(sqle, "SQL Exception:" + sqle.getMessage(), MODULE);
262-
throw sqle;
263-
} finally {
264-
try {
265-
if (stmt != null) stmt.close();
266-
} catch (SQLException sqle) {
267-
Debug.logWarning(sqle, "Error closing statement in sequence util", MODULE);
256+
if (committed) {
257+
Debug.logWarning(sqle, "Error closing database resources in sequence util", MODULE);
258+
} else if (!connectionAvailable) {
259+
Debug.logWarning("Unable to establish a connection with the database. Error was:" + sqle.toString(), MODULE);
260+
throw sqle;
261+
} else {
262+
Debug.logWarning(sqle, "SQL Exception:" + sqle.getMessage(), MODULE);
263+
throw sqle;
268264
}
269-
try {
270-
connection.close();
271-
} catch (SQLException sqle) {
272-
Debug.logWarning(sqle, "Error closing connection in sequence util", MODULE);
265+
} catch (GenericEntityException e) {
266+
if (!connectionAvailable) {
267+
Debug.logWarning("Unable to establish a connection with the database. Error was: " + e.toString(), MODULE);
273268
}
269+
throw e;
274270
}
275271
} catch (SQLException | GenericEntityException e) {
276272
// reset the sequence fields and return (note: it would be better to throw an exception)
@@ -313,4 +309,3 @@ private void fillBank(long stagger) {
313309
}
314310
}
315311
}
316-

0 commit comments

Comments
 (0)