Update cloudsql/ samples to address spotbugs issues. (GoogleCloudPlatform#2678)

* Fix lint errors. * Lint fixes for postgres. * Update maven plugin to use gcloud config.

Author: kurtisvg

cloud-sql/mysql/servlet/pom.xml

@@ -27,7 +27,7 @@
  <parent>
  <groupId>com.google.cloud.samples</groupId>
  <artifactId>shared-configuration</artifactId>
- <version>1.0.15</version>
+ <version>1.0.16</version>
  </parent>
 
  <properties>
@@ -91,6 +91,10 @@
  <groupId>com.google.cloud.tools</groupId>
  <artifactId>appengine-maven-plugin</artifactId>
  <version>2.2.0</version>
+ <configuration>
+ <deploy.projectId>GCLOUD_CONFIG</deploy.projectId>
+ <deploy.version>GCLOUD_CONFIG</deploy.version>
+ </configuration>
  </plugin>
  </plugins>
  </build>

cloud-sql/mysql/servlet/src/main/java/com/example/cloudsql/ConnectionPoolContextListener.java

@@ -18,28 +18,33 @@
 
 import com.zaxxer.hikari.HikariConfig;
 import com.zaxxer.hikari.HikariDataSource;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
-import java.util.logging.Logger;
+import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 import javax.servlet.annotation.WebListener;
 import javax.sql.DataSource;
 
+@SuppressFBWarnings(
+ value = {"HARD_CODE_PASSWORD", "WEM_WEAK_EXCEPTION_MESSAGING"},
+ justification = "Extracted from environment, Exception message adds context.")
 @WebListener("Creates a connection pool that is stored in the Servlet's context for later use.")
 public class ConnectionPoolContextListener implements ServletContextListener {
 
- private static final Logger LOGGER = Logger.getLogger(IndexServlet.class.getName());
-
  // Saving credentials in environment variables is convenient, but not secure - consider a more
  // secure solution such as https://cloud.google.com/kms/ to help keep secrets safe.
- private static final String CLOUD_SQL_CONNECTION_NAME = System.getenv(
- "CLOUD_SQL_CONNECTION_NAME");
+ private static final String CLOUD_SQL_CONNECTION_NAME =
+ System.getenv("CLOUD_SQL_CONNECTION_NAME");
  private static final String DB_USER = System.getenv("DB_USER");
  private static final String DB_PASS = System.getenv("DB_PASS");
  private static final String DB_NAME = System.getenv("DB_NAME");
 
+ @SuppressFBWarnings(
+ value = "USBR_UNNECESSARY_STORE_BEFORE_RETURN",
+ justification = "Necessary for sample region tag.")
  private DataSource createConnectionPool() {
  // [START cloud_sql_mysql_servlet_create]
  // The configuration object specifies behaviors for the connection pool.
@@ -54,7 +59,6 @@ private DataSource createConnectionPool() {
  // See https://github.com/GoogleCloudPlatform/cloud-sql-jdbc-socket-factory for details.
  config.addDataSourceProperty("socketFactory", "com.google.cloud.sql.mysql.SocketFactory");
  config.addDataSourceProperty("cloudSqlInstance", CLOUD_SQL_CONNECTION_NAME);
- config.addDataSourceProperty("useSSL", "false");
 
  // ... Specify additional connection properties here.
  // [START_EXCLUDE]
@@ -102,12 +106,13 @@ private DataSource createConnectionPool() {
  private void createTable(DataSource pool) throws SQLException {
  // Safely attempt to create the table schema.
  try (Connection conn = pool.getConnection()) {
- PreparedStatement createTableStatement = conn.prepareStatement(
+ String stmt =
  "CREATE TABLE IF NOT EXISTS votes ( "
  + "vote_id SERIAL NOT NULL, time_cast timestamp NOT NULL, candidate CHAR(6) NOT NULL,"
- + " PRIMARY KEY (vote_id) );"
- );
- createTableStatement.execute();
+ + " PRIMARY KEY (vote_id) );";
+ try (PreparedStatement createTableStatement = conn.prepareStatement(stmt); ) {
+ createTableStatement.execute();
+ }
  }
  }
 
@@ -124,16 +129,19 @@ public void contextDestroyed(ServletContextEvent event) {
  public void contextInitialized(ServletContextEvent event) {
  // This function is called when the application starts and will safely create a connection pool
  // that can be used to connect to.
- DataSource pool = (DataSource) event.getServletContext().getAttribute("my-pool");
+ ServletContext servletContext = event.getServletContext();
+ DataSource pool = (DataSource) servletContext.getAttribute("my-pool");
  if (pool == null) {
  pool = createConnectionPool();
- event.getServletContext().setAttribute("my-pool", pool);
+ servletContext.setAttribute("my-pool", pool);
  }
  try {
  createTable(pool);
  } catch (SQLException ex) {
- throw new RuntimeException("Unable to verify table schema. Please double check the steps"
- + "in the README and try again.", ex);
+ throw new RuntimeException(
+ "Unable to verify table schema. Please double check the steps"
+ + "in the README and try again.",
+ ex);
  }
  }
 }

cloud-sql/mysql/servlet/src/main/java/com/example/cloudsql/IndexServlet.java

@@ -16,6 +16,7 @@
 
 package com.example.cloudsql;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
@@ -25,15 +26,20 @@
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
+import java.util.Locale;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import javax.servlet.ServletException;
 import javax.servlet.annotation.WebServlet;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.sql.DataSource;
 
+@SuppressFBWarnings(
+ value = {"SE_NO_SERIALVERSIONID", "WEM_WEAK_EXCEPTION_MESSAGING"},
+ justification = "Not needed for IndexServlet, Exception adds context")
 @WebServlet(name = "Index", value = "")
 public class IndexServlet extends HttpServlet {
 
@@ -46,44 +52,48 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp)
  // in the ContextListener when the application was started
  DataSource pool = (DataSource) req.getServletContext().getAttribute("my-pool");
 
- int tabCount;
- int spaceCount;
+ int tabCount = 0;
+ int spaceCount = 0;
  List<Vote> recentVotes = new ArrayList<>();
  try (Connection conn = pool.getConnection()) {
  // PreparedStatements are compiled by the database immediately and executed at a later date.
  // Most databases cache previously compiled queries, which improves efficiency.
- PreparedStatement voteStmt = conn.prepareStatement(
- "SELECT candidate, time_cast FROM votes ORDER BY time_cast DESC LIMIT 5");
- // Execute the statement
- ResultSet voteResults = voteStmt.executeQuery();
- // Convert a ResultSet into Vote objects
- while (voteResults.next()) {
- String candidate = voteResults.getString(1);
- Timestamp timeCast = voteResults.getTimestamp(2);
- recentVotes.add(new Vote(candidate, timeCast));
+ String stmt1 = "SELECT candidate, time_cast FROM votes ORDER BY time_cast DESC LIMIT 5";
+ try (PreparedStatement voteStmt = conn.prepareStatement(stmt1); ) {
+ // Execute the statement
+ ResultSet voteResults = voteStmt.executeQuery();
+ // Convert a ResultSet into Vote objects
+ while (voteResults.next()) {
+ String candidate = voteResults.getString(1);
+ Timestamp timeCast = voteResults.getTimestamp(2);
+ recentVotes.add(new Vote(candidate, timeCast));
+ }
  }
 
  // PreparedStatements can also be executed multiple times with different arguments. This can
  // improve efficiency, and project a query from being vulnerable to an SQL injection.
- PreparedStatement voteCountStmt = conn.prepareStatement(
- "SELECT COUNT(vote_id) FROM votes WHERE candidate=?");
-
- voteCountStmt.setString(1, "TABS");
- ResultSet tabResult = voteCountStmt.executeQuery();
- tabResult.next(); // Move to the first result
- tabCount = tabResult.getInt(1);
-
- voteCountStmt.setString(1, "SPACES");
- ResultSet spaceResult = voteCountStmt.executeQuery();
- spaceResult.next(); // Move to the first result
- spaceCount = spaceResult.getInt(1);
-
+ String stmt2 = "SELECT COUNT(vote_id) FROM votes WHERE candidate=?";
+ try (PreparedStatement voteCountStmt = conn.prepareStatement(stmt2); ) {
+ voteCountStmt.setString(1, "TABS");
+ ResultSet tabResult = voteCountStmt.executeQuery();
+ if (tabResult.next()) { // Move to the first result
+ tabCount = tabResult.getInt(1);
+ }
+
+ voteCountStmt.setString(1, "SPACES");
+ ResultSet spaceResult = voteCountStmt.executeQuery();
+ if (spaceResult.next()) { // Move to the first result
+ spaceCount = spaceResult.getInt(1);
+ }
+ }
  } catch (SQLException ex) {
  // If something goes wrong, the application needs to react appropriately. This might mean
  // getting a new connection and executing the query again, or it might mean redirecting the
  // user to a different page to let them know something went wrong.
- throw new ServletException("Unable to successfully connect to the database. Please check the "
- + "steps in the README and try again.", ex);
+ throw new ServletException(
+ "Unable to successfully connect to the database. Please check the "
+ + "steps in the README and try again.",
+ ex);
  }
 
  // Add variables and render the page
@@ -93,16 +103,29 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp)
  req.getRequestDispatcher("/index.jsp").forward(req, resp);
  }
 
+ // Used to validate user input. All user provided data should be validated and sanitized before
+ // being used something like a SQL query. Returns null if invalid.
+ @Nullable
+ private String validateTeam(String input) {
+ if (input != null) {
+ input = input.toUpperCase(Locale.ENGLISH);
+ // Must be either "TABS" or "SPACES"
+ if (!"TABS".equals(input) && !"SPACES".equals(input)) {
+ return null;
+ }
+ }
+ return input;
+ }
+
+ @SuppressFBWarnings(
+ value = {"SERVLET_PARAMETER", "XSS_SERVLET"},
+ justification = "Input is validated and sanitized.")
  @Override
- public void doPost(HttpServletRequest req, HttpServletResponse resp)
- throws IOException {
+ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
  // Get the team from the request and record the time of the vote.
- String team = req.getParameter("team");
- if (team != null) {
- team = team.toUpperCase();
- }
+ String team = validateTeam(req.getParameter("team"));
  Timestamp now = new Timestamp(new Date().getTime());
- if (team == null || (!team.equals("TABS") && !team.equals("SPACES"))) {
+ if (team == null) {
  resp.setStatus(400);
  resp.getWriter().append("Invalid team specified.");
  return;
@@ -116,28 +139,29 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp)
  try (Connection conn = pool.getConnection()) {
 
  // PreparedStatements can be more efficient and project against injections.
- PreparedStatement voteStmt = conn.prepareStatement(
- "INSERT INTO votes (time_cast, candidate) VALUES (?, ?);");
- voteStmt.setTimestamp(1, now);
- voteStmt.setString(2, team);
-
- // Finally, execute the statement. If it fails, an error will be thrown.
- voteStmt.execute();
+ String stmt = "INSERT INTO votes (time_cast, candidate) VALUES (?, ?);";
+ try (PreparedStatement voteStmt = conn.prepareStatement(stmt); ) {
+ voteStmt.setTimestamp(1, now);
+ voteStmt.setString(2, team);
 
+ // Finally, execute the statement. If it fails, an error will be thrown.
+ voteStmt.execute();
+ }
  } catch (SQLException ex) {
  // If something goes wrong, handle the error in this section. This might involve retrying or
  // adjusting parameters depending on the situation.
  // [START_EXCLUDE]
  LOGGER.log(Level.WARNING, "Error while attempting to submit vote.", ex);
  resp.setStatus(500);
- resp.getWriter().write("Unable to successfully cast vote! Please check the application "
- + "logs for more details.");
+ resp.getWriter()
+ .write(
+ "Unable to successfully cast vote! Please check the application "
+ + "logs for more details.");
  // [END_EXCLUDE]
  }
  // [END cloud_sql_mysql_servlet_connection]
 
  resp.setStatus(200);
- resp.getWriter().printf("Vote successfully cast for '%s' at time %s!\n", team, now);
+ resp.getWriter().printf("Vote successfully cast for '%s' at time %s!%n", team, now);
  }
-
 }

cloud-sql/mysql/servlet/src/main/java/com/example/cloudsql/Vote.java

@@ -17,31 +17,35 @@
 package com.example.cloudsql;
 
 import java.sql.Timestamp;
+import java.util.Locale;
 
 public class Vote {
 
  private String candidate;
  private Timestamp timeCast;
 
  public Vote(String candidate, Timestamp timeCast) {
- this.candidate = candidate.toUpperCase();
- this.timeCast = timeCast;
+ this.candidate = candidate.toUpperCase(Locale.ENGLISH);
+ this.timeCast = new Timestamp(timeCast.getTime());
  }
 
  public String getCandidate() {
  return candidate;
  }
 
  public void setCandidate(String candidate) {
- this.candidate = candidate.toUpperCase();
+ this.candidate = candidate.toUpperCase(Locale.ENGLISH);
  }
 
  public Timestamp getTimeCast() {
- return timeCast;
+ return new Timestamp(timeCast.getTime());
  }
 
  public void setTimeCast(Timestamp timeCast) {
- this.timeCast = timeCast;
+ this.timeCast = new Timestamp(timeCast.getTime());
  }
 
+ public String toString() {
+ return String.format("Vote(candidate=%s,timeCast=%s)", this.candidate, this.timeCast);
+ }
 }

cloud-sql/postgres/servlet/pom.xml

@@ -27,7 +27,7 @@
  <parent>
  <groupId>com.google.cloud.samples</groupId>
  <artifactId>shared-configuration</artifactId>
- <version>1.0.15</version>
+ <version>1.0.16</version>
  </parent>
 
  <properties>
@@ -81,6 +81,10 @@
  <groupId>com.google.cloud.tools</groupId>
  <artifactId>appengine-maven-plugin</artifactId>
  <version>2.2.0</version>
+ <configuration>
+ <deploy.projectId>GCLOUD_CONFIG</deploy.projectId>
+ <deploy.version>GCLOUD_CONFIG</deploy.version>
+ </configuration>
  </plugin>
  </plugins>
  </build>