From 5ff57e9587e701312d4a48fb209bc471f277c303 Mon Sep 17 00:00:00 2001
From: Gaofei Zhao <15748980+dippindots@users.noreply.github.com>
Date: Wed, 27 Nov 2024 11:01:23 -0500
Subject: [PATCH] Use prepared statements to avoid injection attack and
 clickhouse native array to improve performance

---
 .../helper/StudyViewFilterHelper.java         | 12 ++++++++++++
 .../web/parameter/CustomSampleIdentifier.java | 12 ++++++++++++
 .../StudyViewFilterMapper.xml                 | 19 +++++++++++--------
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/cbioportal/persistence/helper/StudyViewFilterHelper.java b/src/main/java/org/cbioportal/persistence/helper/StudyViewFilterHelper.java
index 23615be0ef4..e783e684a82 100644
--- a/src/main/java/org/cbioportal/persistence/helper/StudyViewFilterHelper.java
+++ b/src/main/java/org/cbioportal/persistence/helper/StudyViewFilterHelper.java
@@ -55,6 +55,7 @@ public static StudyViewFilterHelper build(@Nullable StudyViewFilter studyViewFil
     private final StudyViewFilter studyViewFilter;
     private final CategorizedGenericAssayDataCountFilter categorizedGenericAssayDataCountFilter;
     private final List<CustomSampleIdentifier> customDataSamples;
+    private final String[] filteredSampleIdentifiers;
     private final List<String> involvedCancerStudies;
 
     private StudyViewFilterHelper(@NonNull StudyViewFilter studyViewFilter, 
@@ -65,6 +66,13 @@ private StudyViewFilterHelper(@NonNull StudyViewFilter studyViewFilter,
         this.categorizedGenericAssayDataCountFilter = extractGenericAssayDataCountFilters(studyViewFilter, genericAssayProfilesMap);
         this.customDataSamples = customDataSamples;
         this.involvedCancerStudies = involvedCancerStudies;
+        if (studyViewFilter != null && studyViewFilter.getSampleIdentifiers() != null) {
+            this.filteredSampleIdentifiers = studyViewFilter.getSampleIdentifiers().stream()
+                .map(sampleIdentifier -> sampleIdentifier.getStudyId() + "_" + sampleIdentifier.getSampleId())
+                .toArray(String[]::new);
+        } else {
+            this.filteredSampleIdentifiers = new String[0];
+        }
     }
 
     public StudyViewFilter studyViewFilter() {
@@ -79,6 +87,10 @@ public List<CustomSampleIdentifier> customDataSamples() {
         return this.customDataSamples;
     }
     
+    public String[] filteredSampleIdentifiers() {
+        return this.filteredSampleIdentifiers;
+    }
+    
     public List<String> involvedCancerStudies() {
         return involvedCancerStudies;
     }
diff --git a/src/main/java/org/cbioportal/web/parameter/CustomSampleIdentifier.java b/src/main/java/org/cbioportal/web/parameter/CustomSampleIdentifier.java
index 9f5f4e0bfe0..0eade688034 100644
--- a/src/main/java/org/cbioportal/web/parameter/CustomSampleIdentifier.java
+++ b/src/main/java/org/cbioportal/web/parameter/CustomSampleIdentifier.java
@@ -6,6 +6,8 @@ public class CustomSampleIdentifier extends SampleIdentifier implements Serializ
     
     private boolean isFilteredOut = false;
     private String value;
+    
+    private String uniqueSampleId;
 
     public boolean getIsFilteredOut() {
         return isFilteredOut;
@@ -22,4 +24,14 @@ public String getValue() {
     public void setValue(String value) {
         this.value = value;
     }
+
+    // Generating unique SampleId by concatenating studyId and sampleId
+    public String getUniqueSampleId() {
+        // Assuming studyId and sampleId are available in SampleIdentifier
+        // Concatenate with "_" in between if both values are not null
+        if (getStudyId() != null && getSampleId() != null) {
+            return getStudyId() + "_" + getSampleId();
+        }
+        return null;  // or return a default value if either studyId or sampleId is null
+    }
 }
diff --git a/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewFilterMapper.xml b/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewFilterMapper.xml
index 584f8cf95bd..584f24e87a1 100644
--- a/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewFilterMapper.xml
+++ b/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewFilterMapper.xml
@@ -61,14 +61,14 @@
 
             </if>
 
-            <if test="studyViewFilterHelper.studyViewFilter.sampleIdentifiers != null and !studyViewFilterHelper.studyViewFilter.sampleIdentifiers.isEmpty()">
-                INTERSECT
+            <if test="studyViewFilterHelper.filteredSampleIdentifiers != null and studyViewFilterHelper.filteredSampleIdentifiers.length > 0">
+            INTERSECT
                 SELECT sample_unique_id
                 FROM sample_derived
                 WHERE sample_unique_id IN
-                <foreach item="sampleIdentifier" collection="studyViewFilterHelper.studyViewFilter.sampleIdentifiers" open="(" separator="," close=")">
-                    '${sampleIdentifier.studyId}_${sampleIdentifier.sampleId}'
-                </foreach>
+                (
+                    #{studyViewFilterHelper.filteredSampleIdentifiers, typeHandler=org.apache.ibatis.type.ArrayTypeHandler}
+                )
             </if>
             <if test="studyViewFilterHelper.studyViewFilter.customDataFilters != null and !studyViewFilterHelper.studyViewFilter.customDataFilters.isEmpty() and studyViewFilterHelper.customDataSamples != null">
                 INTERSECT
@@ -86,8 +86,8 @@
                             sample_unique_id IN (
                                 '',
                                 <foreach item="sampleIdentifier" collection="studyViewFilterHelper.customDataSamples" separator=",">
-                                    <if test="!sampleIdentifier.getIsFilteredOut()">
-                                        '${sampleIdentifier.studyId}_${sampleIdentifier.sampleId}'
+                                    <if test="!sampleIdentifier.getIsFilteredOut() and sampleIdentifier.getUniqueSampleId() != null">
+                                        #{sampleIdentifier.uniqueSampleId}
                                     </if>
                                 </foreach>
                             )
@@ -97,8 +97,11 @@
                                 <if test="customDataFilterValue.value eq 'NA'">
                                     OR
                                     sample_unique_id NOT IN (
+                                        '',
                                         <foreach item="sampleIdentifier" collection="studyViewFilterHelper.customDataSamples" separator=",">
-                                            '${sampleIdentifier.studyId}_${sampleIdentifier.sampleId}'
+                                            <if test="sampleIdentifier.getUniqueSampleId() != null">
+                                                #{sampleIdentifier.uniqueSampleId}
+                                            </if>
                                         </foreach>
                                     )
                                 </if>