[ 
https://issues.apache.org/jira/browse/PHOENIX-6560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17711282#comment-17711282
 ] 

ASF GitHub Bot commented on PHOENIX-6560:
-----------------------------------------

kabhishek4 commented on code in PR #1586:
URL: https://github.com/apache/phoenix/pull/1586#discussion_r1163904950


##########
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java:
##########
@@ -2163,45 +2210,53 @@ public static void 
upgradeDescVarLengthRowKeys(PhoenixConnection conn, List<Stri
      * @return true if any upgrades were performed and false otherwise.
      */
     private static boolean upgradeSharedIndex(PhoenixConnection upgradeConn, 
PhoenixConnection globalConn, String physicalName, boolean bypassUpgrade) 
throws SQLException {
-        String query =
-                "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME\n" + 
-                "FROM SYSTEM.CATALOG cat1\n" + 
-                "WHERE COLUMN_NAME IS NULL\n" + 
-                "AND COLUMN_FAMILY = '" + physicalName + "'\n" + 
-                "AND LINK_TYPE = " + 
LinkType.PHYSICAL_TABLE.getSerializedValue() + "\n" +
-                "ORDER BY TENANT_ID";
-        ResultSet rs = globalConn.createStatement().executeQuery(query);
-        String lastTenantId = null;
-        Connection conn = globalConn;
-        String url = globalConn.getURL();
-        boolean wasUpgraded = false;
-        while (rs.next()) {
-            String fullTableName = SchemaUtil.getTableName(
+        String query = String.format(
+            "SELECT TENANT_ID,TABLE_SCHEM,TABLE_NAME"
+                + "FROM SYSTEM.CATALOG cat1"
+                + "WHERE COLUMN_NAME IS NULL"
+                + "AND COLUMN_FAMILY = ? "
+                + "AND LINK_TYPE = ? "
+                + "ORDER BY TENANT_ID", physicalName,
+            LinkType.PHYSICAL_TABLE.getSerializedValue());
+        try (PreparedStatement selSysCatstmt = 
globalConn.prepareStatement(query)) {
+            selSysCatstmt.setString(1, physicalName);
+            selSysCatstmt.setByte(2, 
LinkType.PHYSICAL_TABLE.getSerializedValue());
+            ResultSet rs = selSysCatstmt.executeQuery();
+            String lastTenantId = null;
+            Connection conn = globalConn;
+            String url = globalConn.getURL();
+            boolean wasUpgraded = false;
+            while (rs.next()) {
+                String fullTableName = SchemaUtil.getTableName(
                     rs.getString(PhoenixDatabaseMetaData.TABLE_SCHEM),
                     rs.getString(PhoenixDatabaseMetaData.TABLE_NAME));
-            String tenantId = rs.getString(1);
-            if (tenantId != null && !tenantId.equals(lastTenantId))  {
-                if (lastTenantId != null) {
-                    conn.close();
+                String tenantId = rs.getString(1);
+                if (tenantId != null && !tenantId.equals(lastTenantId)) {
+                    if (lastTenantId != null) {
+                        conn.close();
+                    }
+                    // Open tenant-specific connection when we find a new one
+                    Properties props = new 
Properties(globalConn.getClientInfo());
+                    props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, 
tenantId);
+                    conn = DriverManager.getConnection(url, props);
+                    lastTenantId = tenantId;
+                }
+                PTable table = PhoenixRuntime.getTable(conn, fullTableName);
+                String tableTenantId =
+                    table.getTenantId() == null ? null : 
table.getTenantId().getString();
+                if (Objects.equal(lastTenantId, tableTenantId) && 
!table.rowKeyOrderOptimizable()) {
+                    upgradeDescVarLengthRowKeys(upgradeConn, globalConn,
+                        table.getSchemaName().getString(), 
table.getTableName().getString(), false,
+                        bypassUpgrade);
+                    wasUpgraded = true;
                 }
-                // Open tenant-specific connection when we find a new one
-                Properties props = new Properties(globalConn.getClientInfo());
-                props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId);
-                conn = DriverManager.getConnection(url, props);
-                lastTenantId = tenantId;
             }
-            PTable table = PhoenixRuntime.getTable(conn, fullTableName);
-            String tableTenantId = table.getTenantId() == null ? null : 
table.getTenantId().getString();
-            if (Objects.equal(lastTenantId, tableTenantId) && 
!table.rowKeyOrderOptimizable()) {
-                upgradeDescVarLengthRowKeys(upgradeConn, globalConn, 
table.getSchemaName().getString(), table.getTableName().getString(), false, 
bypassUpgrade);
-                wasUpgraded = true;
+            rs.close();
+            if (lastTenantId != null) {
+                conn.close();

Review Comment:
   This has already been added into try-with-resource block which starts at 
line 2223.
   





> Rewrite dynamic SQL queries to use Preparedstatement
> ----------------------------------------------------
>
>                 Key: PHOENIX-6560
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-6560
>             Project: Phoenix
>          Issue Type: Improvement
>          Components: core
>            Reporter: Istvan Toth
>            Assignee: Abhishek Kothalikar
>            Priority: Major
>
> Most of the Phoenix code base already uses PreparedStatements, and adds all 
> potentially vulnerable data as parameters.
> However, there are some places where we concatenate potentially problematic 
> strings into the query.
> While most of those are constants and such, we should preferably pass all 
> data as parameters to be on the safe side.
> (We still have to use dynamic strings for the preparedstatement strings, for 
> handling things as is null, empty in clauses and such)
> Spotbugs marks these with SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE, so 
> they're easy to find.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to