Skip to content

Commit fcbe980

Browse files
labkey-adamlabkey-adam
authored
Use helper methods to create IN clauses (#1087)
## Rationale Using helpers to generate SQL IN clauses is more convenient and prevents broken SQL & injection risks ## Related Pull Requests - LabKey/platform#8187
1 parent e10385d commit fcbe980

25 files changed

Lines changed: 125 additions & 292 deletions

File tree

‎elisa/src/org/labkey/elisa/plate/BioTekPlateReader.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.labkey.elisa.plate;
1717

18-
import org.apache.commons.lang3.StringUtils;
18+
import org.apache.commons.lang3.Strings;
1919
import org.apache.poi.ss.usermodel.Cell;
2020
import org.apache.poi.ss.usermodel.CellType;
2121
import org.apache.poi.ss.usermodel.Row;
@@ -43,7 +43,7 @@ protected boolean isValidStartRow(Sheet sheet, int row)
4343
{
4444
for (Cell cell : sheetRow)
4545
{
46-
if (cell.getCellType() == CellType.STRING && StringUtils.equalsIgnoreCase(cell.getStringCellValue(), "A"))
46+
if (cell.getCellType() == CellType.STRING && Strings.CI.equals(cell.getStringCellValue(), "A"))
4747
continue;
4848
else if (cell.getCellType() != CellType.NUMERIC)
4949
return false;

‎flow/enginesrc/org/labkey/flow/analysis/model/FlowJoWorkspace.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.labkey.flow.analysis.model;
1818

1919
import org.apache.commons.lang3.StringUtils;
20+
import org.apache.commons.lang3.Strings;
2021
import org.apache.logging.log4j.Logger;
2122
import org.jetbrains.annotations.Nullable;
2223
import org.junit.Assert;
@@ -392,8 +393,8 @@ protected double getRange(Element elParameter)
392393

393394
static public String ___cleanName(String name)
394395
{
395-
name = StringUtils.replace(name, "<", CompensationMatrix.PREFIX);
396-
name = StringUtils.replace(name, ">", CompensationMatrix.SUFFIX);
396+
name = Strings.CS.replace(name, "<", CompensationMatrix.PREFIX);
397+
name = Strings.CS.replace(name, ">", CompensationMatrix.SUFFIX);
397398
name = StringUtils.replaceChars(name, ',', ';');
398399
// replace MacRoman em-dash and en-dash
399400
name = StringUtils.replaceChars(name, PopulationName.MAC_EMDASH, '-');

‎flow/src/org/labkey/flow/ScriptParser.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.labkey.flow;
1717

18-
import org.apache.commons.lang3.StringUtils;
18+
import org.apache.commons.lang3.Strings;
1919
import org.apache.xmlbeans.XmlError;
2020
import org.apache.xmlbeans.XmlException;
2121
import org.apache.xmlbeans.XmlOptions;
@@ -83,7 +83,7 @@ public void parse(String script)
8383
XmlOptions options = new XmlOptions();
8484
List<XmlError> errors = new ArrayList<>();
8585
options.setDocumentType(ScriptDocument.type);
86-
script = StringUtils.replace(script, "<script>", "<script xmlns=\"" + ScriptDocument.type.getContentModel().getName().getNamespaceURI() + "\">");
86+
script = Strings.CS.replace(script, "<script>", "<script xmlns=\"" + ScriptDocument.type.getContentModel().getName().getNamespaceURI() + "\">");
8787
ScriptDocument doc = ScriptDocument.Factory.parse(new StringReader(script), options);
8888
options.setErrorListener(errors);
8989

@@ -92,7 +92,7 @@ public void parse(String script)
9292
for (XmlError xmlError : errors)
9393
{
9494
String message = xmlError.getMessage();
95-
message = StringUtils.replace(message, "@" + ScriptDocument.type.getContentModel().getName().getNamespaceURI(), "");
95+
message = Strings.CS.replace(message, "@" + ScriptDocument.type.getContentModel().getName().getNamespaceURI(), "");
9696
String location = xmlError.getCursorLocation().xmlText();
9797
if (location.length() > 100)
9898
location = StringUtilsLabKey.leftSurrogatePairFriendly(location, 100);

‎flow/src/org/labkey/flow/controllers/executescript/ChooseRunsToAnalyzeForm.java‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -226,26 +226,17 @@ public SimpleFilter getBaseFilter(TableInfo table, Filter filter)
226226
SimpleFilter ret = new SimpleFilter(filter);
227227

228228
List<FlowRun> runs = FlowRun.getRunsForContainer(getContainer(), FlowProtocolStep.keywords);
229-
int runCount = 0;
230-
SQLFragment sql = new SQLFragment("RowId IN (");
231-
String comma = "";
232-
for (FlowRun run : runs)
233-
{
234-
if (run.getPath() == null)
235-
continue;
236-
sql.append(comma);
237-
comma = ",";
238-
sql.appendValue(run.getRunId());
239-
runCount ++;
240-
}
241-
sql.append(")");
242-
if (runCount == 0)
229+
List<Long> runIds = runs.stream()
230+
.filter(run -> run.getPath() != null)
231+
.map(FlowRun::getRunId)
232+
.toList();
233+
if (runIds.isEmpty())
243234
{
244235
ret.addWhereClause(new SQLFragment("1 = 0"));
245236
}
246237
else
247238
{
248-
ret.addWhereClause(sql, FieldKey.fromParts("RowId"));
239+
ret.addInClause(FieldKey.fromParts("RowId"), runIds);
249240
}
250241
return ret;
251242
}

‎flow/src/org/labkey/flow/controllers/well/WellController.java‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,13 @@
105105
import java.io.IOException;
106106
import java.net.URI;
107107
import java.util.ArrayList;
108+
import java.util.Arrays;
108109
import java.util.Collection;
109110
import java.util.Collections;
110111
import java.util.HashSet;
111112
import java.util.List;
112113
import java.util.Map;
114+
import java.util.Objects;
113115
import java.util.Set;
114116
import java.util.TreeSet;
115117

@@ -756,18 +758,8 @@ public boolean handlePost(UpdateKeywordsForm form, BindException errors)
756758
update.add(getContainer());
757759
update.add(ObjectType.fcsKeywords.getTypeId());
758760
update.add(_keywordid);
759-
update.append(" AND value IN (");
760-
String param = "?";
761-
for (int i=0 ; i<form.from.length ; i++)
762-
{
763-
if (null != form.from[i])
764-
{
765-
update.append(param);
766-
update.add(form.from[i]);
767-
param = ",?";
768-
}
769-
}
770-
update.append(")");
761+
update.append(" AND value");
762+
update.appendInClause(Arrays.stream(form.from).filter(Objects::nonNull).toList(), FlowManager.get().getSchema().getSqlDialect());
771763

772764
int updated = new SqlExecutor(FlowManager.get().getSchema()).execute(update);
773765

‎flow/src/org/labkey/flow/data/FieldSubstitution.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
package org.labkey.flow.data;
1818

19-
import org.apache.commons.lang3.StringUtils;
19+
import org.apache.commons.lang3.Strings;
2020
import org.apache.logging.log4j.Logger;
2121
import org.apache.logging.log4j.LogManager;
2222
import org.labkey.api.query.FieldKey;
@@ -38,10 +38,10 @@ static public FieldSubstitution fromString(String s)
3838
int ichCur = 0;
3939
while (true)
4040
{
41-
int ichExprStart = StringUtils.indexOf(s, "${", ichCur);
41+
int ichExprStart = Strings.CS.indexOf(s, "${", ichCur);
4242
if (ichExprStart < 0)
4343
break;
44-
int ichExprEnd = StringUtils.indexOf(s, "}", ichExprStart);
44+
int ichExprEnd = Strings.CS.indexOf(s, "}", ichExprStart);
4545
if (ichExprEnd < 0)
4646
break;
4747
if (ichExprStart > ichCur)

‎flow/src/org/labkey/flow/persist/AttributeSetHelper.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@
1515
*/
1616
package org.labkey.flow.persist;
1717

18-
import org.apache.commons.lang3.StringUtils;
1918
import org.apache.logging.log4j.LogManager;
2019
import org.apache.logging.log4j.Logger;
2120
import org.jetbrains.annotations.Nullable;
2221
import org.labkey.api.collections.LongHashMap;
2322
import org.labkey.api.data.Container;
2423
import org.labkey.api.data.DbScope;
24+
import org.labkey.api.data.SQLFragment;
2525
import org.labkey.api.data.SqlSelector;
2626
import org.labkey.api.data.Table;
2727
import org.labkey.api.exp.api.ExpData;
@@ -254,12 +254,12 @@ private static void loadFromDb(final AttributeSet attrs, AttrObject obj, final b
254254

255255
if (!keywordIDs.isEmpty())
256256
{
257-
String sqlKeywordAliaes = "SELECT A.name AS PreferredName, B.Name AS AliasName\n" +
257+
SQLFragment sqlKeywordAliaes = new SQLFragment("SELECT A.name AS PreferredName, B.Name AS AliasName\n" +
258258
"FROM flow.KeywordAttr A\n" +
259259
"INNER JOIN flow.KeywordAttr B\n" +
260260
"ON A.RowId = B.Id\n" +
261261
"AND B.RowId != B.Id\n" +
262-
"AND A.RowId IN (" + StringUtils.join(keywordIDs, ", ") + ")";
262+
"AND A.RowId").appendInClause(keywordIDs, mgr.getSchema().getSqlDialect());
263263

264264
new SqlSelector(mgr.getSchema(), sqlKeywordAliaes).forEach(rs -> attrs.addKeywordAlias(rs.getString(1), rs.getString(2)));
265265
}
@@ -282,12 +282,12 @@ private static void loadFromDb(final AttributeSet attrs, AttrObject obj, final b
282282

283283
if (!statisticIDs.isEmpty())
284284
{
285-
String sqlStatisticAliaes = "SELECT A.name AS PreferredName, B.Name AS AliasName\n" +
285+
SQLFragment sqlStatisticAliaes = new SQLFragment("SELECT A.name AS PreferredName, B.Name AS AliasName\n" +
286286
"FROM flow.StatisticAttr A\n" +
287287
"INNER JOIN flow.StatisticAttr B\n" +
288288
"ON A.RowId = B.Id\n" +
289289
"AND B.RowId != B.Id\n" +
290-
"AND A.RowId IN (" + StringUtils.join(statisticIDs, ", ") + ")";
290+
"AND A.RowId").appendInClause(statisticIDs, mgr.getSchema().getSqlDialect());
291291

292292
new SqlSelector(mgr.getSchema(), sqlStatisticAliaes).forEach(rs -> attrs.addStatisticAlias(new StatisticSpec(rs.getString(1)), new StatisticSpec(rs.getString(2))));
293293
}
@@ -330,12 +330,12 @@ private static void loadFromDb(final AttributeSet attrs, AttrObject obj, final b
330330

331331
if (!graphIDs.isEmpty())
332332
{
333-
String sqlGraphAliaes = "SELECT A.name AS PreferredName, B.Name AS AliasName\n" +
333+
SQLFragment sqlGraphAliaes = new SQLFragment("SELECT A.name AS PreferredName, B.Name AS AliasName\n" +
334334
"FROM flow.GraphAttr A\n" +
335335
"INNER JOIN flow.GraphAttr B\n" +
336336
"ON A.RowId = B.Id\n" +
337337
"AND B.RowId != B.Id\n" +
338-
"AND A.RowId IN (" + StringUtils.join(graphIDs, ", ") + ")";
338+
"AND A.RowId").appendInClause(graphIDs, mgr.getSchema().getSqlDialect());
339339

340340
new SqlSelector(mgr.getSchema(), sqlGraphAliaes).forEach(rs -> attrs.addGraphAlias(new GraphSpec(rs.getString(1)), new GraphSpec(rs.getString(2))));
341341
}

‎flow/src/org/labkey/flow/persist/FlowManager.java‎

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.labkey.flow.persist;
1818

1919
import jakarta.servlet.http.HttpServletResponse;
20-
import org.apache.commons.collections4.iterators.ArrayIterator;
2120
import org.apache.commons.lang3.StringUtils;
2221
import org.apache.logging.log4j.LogManager;
2322
import org.apache.logging.log4j.Logger;
@@ -44,6 +43,7 @@
4443
import org.labkey.api.data.Table;
4544
import org.labkey.api.data.TableInfo;
4645
import org.labkey.api.data.TableSelector;
46+
import org.labkey.api.data.dialect.SqlDialect;
4747
import org.labkey.api.dataiterator.DataIteratorContext;
4848
import org.labkey.api.dataiterator.MapDataIterator;
4949
import org.labkey.api.exp.Handler;
@@ -96,8 +96,6 @@
9696
import java.util.Collections;
9797
import java.util.Date;
9898
import java.util.HashMap;
99-
import java.util.HashSet;
100-
import java.util.Iterator;
10199
import java.util.LinkedHashMap;
102100
import java.util.List;
103101
import java.util.Map;
@@ -953,14 +951,8 @@ public List<AttrObject> getAttrObjects(Collection<ExpData> datas)
953951
{
954952
if (datas.isEmpty())
955953
return Collections.emptyList();
956-
SQLFragment sql = new SQLFragment ("SELECT * FROM " + getTinfoObject().toString() + " WHERE DataId IN (");
957-
String comma = "";
958-
for (ExpData data : datas)
959-
{
960-
sql.append(comma).appendValue(data.getRowId());
961-
comma = ",";
962-
}
963-
sql.append(")");
954+
SQLFragment sql = new SQLFragment ("SELECT * FROM " + getTinfoObject().toString() + " WHERE DataId");
955+
sql.appendInClause(datas.stream().map(ExpData::getRowId).toList(), getSchema().getSqlDialect());
964956
AttrObject[] array = new SqlSelector(getSchema(), sql).getArray(AttrObject.class);
965957
return Arrays.asList(array);
966958
}
@@ -1023,31 +1015,27 @@ public AttrObject createAttrObject(@NotNull ExpData data, ObjectType type, URI u
10231015

10241016
int MAX_BATCH = 1000;
10251017

1026-
private String join(Integer[] oids, int from, int to)
1027-
{
1028-
Iterator<Integer> i = new ArrayIterator<>(oids, from, to);
1029-
return StringUtils.join(i, ',');
1030-
}
1031-
10321018
private void deleteAttributes(Integer[] oids)
10331019
{
10341020
if (oids.length == 0)
10351021
return;
10361022

10371023
SqlExecutor executor = new SqlExecutor(getSchema());
1024+
SqlDialect dialect = getSchema().getSqlDialect();
10381025

10391026
for (int from = 0, to; from < oids.length; from = to)
10401027
{
10411028
to = from + MAX_BATCH;
10421029
if (to > oids.length)
10431030
to = oids.length;
10441031

1045-
String list = join(oids, from, to);
1046-
// XXX: delete no longer referenced statattr afterwards?
1047-
executor.execute("DELETE FROM flow.Statistic WHERE ObjectId IN (" + list + ")");
1048-
executor.execute("DELETE FROM flow.Keyword WHERE ObjectId IN (" + list + ")");
1049-
executor.execute("DELETE FROM flow.Graph WHERE ObjectId IN (" + list + ")");
1050-
executor.execute("DELETE FROM flow.Script WHERE ObjectId IN (" + list + ")");
1032+
List<Integer> list = Arrays.asList(oids).subList(from, to);
1033+
var inList = new SQLFragment().appendInClause(list, dialect);
1034+
// XXX: delete no longer referenced statattr afterward?
1035+
executor.execute(new SQLFragment("DELETE FROM flow.Statistic WHERE ObjectId").append(inList));
1036+
executor.execute(new SQLFragment("DELETE FROM flow.Keyword WHERE ObjectId").append(inList));
1037+
executor.execute(new SQLFragment("DELETE FROM flow.Graph WHERE ObjectId").append(inList));
1038+
executor.execute(new SQLFragment("DELETE FROM flow.Script WHERE ObjectId").append(inList));
10511039
}
10521040
}
10531041

@@ -1069,9 +1057,8 @@ private void deleteObjectIds(Integer[] oids, Set<Container> containers)
10691057
try (DbScope.Transaction transaction = scope.ensureTransaction())
10701058
{
10711059
deleteAttributes(oids);
1072-
SQLFragment sqlf = new SQLFragment("DELETE FROM flow.Object WHERE RowId IN (" );
1073-
sqlf.append(StringUtils.join(oids,','));
1074-
sqlf.append(")");
1060+
SQLFragment sqlf = new SQLFragment("DELETE FROM flow.Object WHERE RowId");
1061+
sqlf.appendInClause(Arrays.asList(oids), getSchema().getSqlDialect());
10751062
new SqlExecutor(getSchema()).execute(sqlf);
10761063
transaction.commit();
10771064
}
@@ -1109,17 +1096,9 @@ public void deleteData(List<ExpData> datas)
11091096
{
11101097
if (datas.isEmpty())
11111098
return;
1112-
StringBuilder sqlGetOIDs = new StringBuilder("SELECT flow.Object.RowId FROM flow.Object WHERE flow.Object.DataId IN (");
1113-
String comma = "";
1114-
Set<Container> containers = new HashSet<>();
1115-
for (ExpData data : datas)
1116-
{
1117-
sqlGetOIDs.append(comma);
1118-
comma = ", ";
1119-
sqlGetOIDs.append(data.getRowId());
1120-
containers.add(data.getContainer());
1121-
}
1122-
sqlGetOIDs.append(")");
1099+
SQLFragment sqlGetOIDs = new SQLFragment("SELECT flow.Object.RowId FROM flow.Object WHERE flow.Object.DataId");
1100+
sqlGetOIDs.appendInClause(datas.stream().map(ExpData::getRowId).toList(), getSchema().getSqlDialect());
1101+
Set<Container> containers = datas.stream().map(ExpData::getContainer).collect(Collectors.toSet());
11231102
Integer[] objectIds = new SqlSelector(getSchema(), sqlGetOIDs).getArray(Integer.class);
11241103
if (objectIds.length == 0)
11251104
return;

‎flow/src/org/labkey/flow/query/FlowSchema.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.labkey.flow.query;
1818

1919
import jakarta.servlet.http.HttpServletRequest;
20-
import org.apache.commons.lang3.StringUtils;
2120
import org.jetbrains.annotations.NotNull;
2221
import org.jetbrains.annotations.Nullable;
2322
import org.labkey.api.assay.AbstractAssayProvider;
@@ -1927,9 +1926,8 @@ private void filterTable(FilteredTable table, Map<? extends Object, Integer> map
19271926
}
19281927
else
19291928
{
1930-
SQLFragment sqlCondition = new SQLFragment("RowId IN (");
1931-
sqlCondition.append(StringUtils.join(map.values().iterator(), ","));
1932-
sqlCondition.append(")");
1929+
SQLFragment sqlCondition = new SQLFragment("RowId");
1930+
sqlCondition.appendInClause(map.values(), table.getSqlDialect());
19331931
table.addCondition(sqlCondition);
19341932
}
19351933
}

‎flow/src/org/labkey/flow/reports/FilterFlowReport.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.fasterxml.jackson.annotation.JsonManagedReference;
2020
import org.apache.commons.beanutils.ConversionException;
2121
import org.apache.commons.lang3.StringUtils;
22+
import org.apache.commons.lang3.Strings;
2223
import org.labkey.api.data.CachedResultSet;
2324
import org.labkey.api.data.CachedResultSetBuilder;
2425
import org.labkey.api.data.ColumnInfo;
@@ -437,7 +438,7 @@ public void updateFilterProperties(PropertyValues pvs)
437438

438439
public String toSQL(String s)
439440
{
440-
return null == s ? "''" : "'" + StringUtils.replace(s, "'", "\'\'") + "'";
441+
return null == s ? "''" : "'" + Strings.CS.replace(s, "'", "\'\'") + "'";
441442
}
442443

443444
public String toSQL(FieldKey fieldKey)

0 commit comments

Comments
 (0)