From 83f40a23d94a0f414a886dbff82a16c846fc6c52 Mon Sep 17 00:00:00 2001 From: LSmyrnaios Date: Thu, 13 Jan 2022 00:54:21 +0200 Subject: [PATCH] Bring back the prepared-statements for the insert-queries. After the fix of the "broken pipe"-error, they now work. Bringing them back, increases security and solves the "SQL syntax errors" caused by the values of some URLs. --- .../controllers/UrlController.java | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/main/java/eu/openaire/urls_controller/controllers/UrlController.java b/src/main/java/eu/openaire/urls_controller/controllers/UrlController.java index 0002e65..d8a90dc 100644 --- a/src/main/java/eu/openaire/urls_controller/controllers/UrlController.java +++ b/src/main/java/eu/openaire/urls_controller/controllers/UrlController.java @@ -9,7 +9,6 @@ import eu.openaire.urls_controller.payloads.responces.AssignmentsResponse; import eu.openaire.urls_controller.util.ControllerConstants; import eu.openaire.urls_controller.util.FileUtils; import eu.openaire.urls_controller.util.GenericUtils; -import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; @@ -308,21 +307,21 @@ public class UrlController { } // Store the workerReport into the database. - String insertIntoPayloadBaseQuery = "INSERT INTO " + ImpalaConnector.databaseName + ".payload (id, original_url, actual_url, `date`, mimetype, size, `hash`, `location`, provenance) VALUES "; - String insertIntoAttemptBaseQuery = "INSERT INTO " + ImpalaConnector.databaseName + ".attempt (id, original_url, `date`, status, error_class, error_message) VALUES "; + String insertIntoPayloadBaseQuery = "INSERT INTO " + ImpalaConnector.databaseName + ".payload (id, original_url, actual_url, date, mimetype, size, hash, location, provenance) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"; + String insertIntoAttemptBaseQuery = "INSERT INTO " + ImpalaConnector.databaseName + ".attempt (id, original_url, date, status, error_class, error_message) VALUES (?, ?, ?, ?, ?, ?)"; String tempInsertQueryName = null; - Statement insertPayloadStatement = null, insertAttemptStatement = null; + PreparedStatement preparedInsertPayloadStatement = null, preparedInsertAttemptStatement = null; try { tempInsertQueryName = "insertIntoPayloadBaseQuery"; - insertPayloadStatement = con.createStatement(); + preparedInsertPayloadStatement = con.prepareStatement(insertIntoPayloadBaseQuery); tempInsertQueryName = "insertIntoAttemptBaseQuery"; - insertAttemptStatement = con.createStatement(); + preparedInsertAttemptStatement = con.prepareStatement(insertIntoAttemptBaseQuery); } catch (SQLException sqle) { ImpalaConnector.databaseLock.unlock(); - String errorMsg = "Problem when creating the statement for \"" + tempInsertQueryName + "\"!\n"; + String errorMsg = "Problem when creating the prepared statement for \"" + tempInsertQueryName + "\"!\n"; logger.error(errorMsg + sqle.getMessage()); - closeStatements(insertPayloadStatement, insertAttemptStatement, con); + closeStatements(preparedInsertPayloadStatement, preparedInsertAttemptStatement, con); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(errorMsg); } @@ -332,7 +331,7 @@ public class UrlController { ImpalaConnector.databaseLock.unlock(); String errorMsg = "Problem when setting Connection.AutoCommit to \"false\"!\n"; logger.error(errorMsg + sqle.getMessage()); - closeStatements(insertPayloadStatement, insertAttemptStatement, con); + closeStatements(preparedInsertPayloadStatement, preparedInsertAttemptStatement, con); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(errorMsg); } @@ -351,20 +350,26 @@ public class UrlController { continue; } - try { + try { // We use a "PreparedStatement" to do insertions, for security and valid SQL syntax reasons. + preparedInsertPayloadStatement.setString(1, payload.getId()); + preparedInsertPayloadStatement.setString(2, payload.getOriginal_url()); + preparedInsertPayloadStatement.setString(3, payload.getActual_url()); + preparedInsertPayloadStatement.setTimestamp(4, payload.getTimestamp_acquired()); + preparedInsertPayloadStatement.setString(5, payload.getMime_type()); + // The column "size" in the table is of type "String" so we cast the Long to String. The Parquet-format in the database does not work well with integers. String stringSize = null; Long size = payload.getSize(); if ( size != null ) stringSize = String.valueOf(size); - String insertIntoPayloadFullQuery = insertIntoPayloadBaseQuery + "('" + payload.getId() + "','" + payload.getOriginal_url() + "','" + payload.getActual_url() + "','" - + payload.getTimestamp_acquired() + "','" + payload.getMime_type() + "','" + stringSize + "','" + payload.getHash() + "','" - + payload.getLocation() + "','" + payload.getProvenance() + "')"; - - insertPayloadStatement.execute(insertIntoPayloadFullQuery); + preparedInsertPayloadStatement.setString(6, stringSize); + preparedInsertPayloadStatement.setString(7, payload.getHash()); + preparedInsertPayloadStatement.setString(8, payload.getLocation()); + preparedInsertPayloadStatement.setString(9, payload.getProvenance()); + preparedInsertPayloadStatement.executeUpdate(); } catch (SQLException sqle) { - logger.error("Problem when executing the \"insertIntoPayloadFullQuery\": " + sqle.getMessage() + "\n\n"); + logger.error("Problem when executing the \"insertIntoPayloadBaseQuery\": " + sqle.getMessage() + "\n\n"); } Error error = urlReport.getError(); @@ -373,15 +378,14 @@ public class UrlController { error = new Error(null, null); } - try { - String errorCause = error.getMessage(); - if ( errorCause != null ) - errorCause = StringUtils.replace(errorCause, "'", "\\'", -1); // Escape single quotes in the error-cause-message. - - String insertIntoAttemptFullQuery = insertIntoAttemptBaseQuery + "('" + payload.getId() + "','" + payload.getOriginal_url() + "','" - + payload.getTimestamp_acquired() + "','" + urlReport.getStatus().toString() + "','" + error.getType() + "','" + errorCause + "')"; - - insertAttemptStatement.execute(insertIntoAttemptFullQuery); + try { // We use a "PreparedStatement" to do insertions, for security and valid SQL syntax reasons. + preparedInsertAttemptStatement.setString(1, payload.getId()); + preparedInsertAttemptStatement.setString(2, payload.getOriginal_url()); + preparedInsertAttemptStatement.setTimestamp(3, payload.getTimestamp_acquired()); + preparedInsertAttemptStatement.setString(4, urlReport.getStatus().toString()); + preparedInsertAttemptStatement.setString(5, String.valueOf(error.getType())); // This covers the case of "null". + preparedInsertAttemptStatement.setString(6, error.getMessage()); + preparedInsertAttemptStatement.executeUpdate(); } catch (SQLException sqle) { logger.error("Problem when executing the \"insertIntoAttemptBaseQuery\": " + sqle.getMessage() + "\n\n"); } @@ -396,7 +400,7 @@ public class UrlController { logger.error(errorMsg + "\n" + sqle.getMessage()); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(errorMsg); } finally { - closeStatements(insertPayloadStatement, insertAttemptStatement, null); // Do not close the connection here! + closeStatements(preparedInsertPayloadStatement, preparedInsertAttemptStatement, null); // Do not close the connection here, as we might move forward. } logger.debug("Finished inserting the payloads and the attempts into the \"payload\" and \"attempt\" tables. Going to merge the parquet files for those tables.");