dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #39756
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 20158: Sql view. Allowing underscore and dash in variables. Proper feedback in response for invalid quer...
------------------------------------------------------------
revno: 20158
committer: Lars Helge Overland <larshelge@xxxxxxxxx>
branch nick: dhis2
timestamp: Tue 2015-09-15 11:43:39 +0200
message:
Sql view. Allowing underscore and dash in variables. Proper feedback in response for invalid query values.
added:
dhis-2/dhis-api/src/test/java/org/hisp/dhis/sqlview/
dhis-2/dhis-api/src/test/java/org/hisp/dhis/sqlview/SqlViewTest.java
modified:
dhis-2/dhis-api/src/main/java/org/hisp/dhis/sqlview/SqlView.java
dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/sqlview/DefaultSqlViewService.java
--
lp:dhis2
https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk
Your team DHIS 2 developers is subscribed to branch lp:dhis2.
To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription
=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/sqlview/SqlView.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/sqlview/SqlView.java 2015-08-03 11:33:39 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/sqlview/SqlView.java 2015-09-15 09:43:39 +0000
@@ -28,11 +28,13 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonView;
-import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
-import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement;
-import com.google.common.collect.Sets;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
+
import org.apache.commons.lang3.StringUtils;
import org.hisp.dhis.common.BaseIdentifiableObject;
import org.hisp.dhis.common.DxfNamespaces;
@@ -44,10 +46,11 @@
import org.hisp.dhis.common.view.ExportView;
import org.hisp.dhis.schema.annotation.PropertyRange;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
-import java.util.regex.Pattern;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonView;
+import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
+import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement;
+import com.google.common.collect.Sets;
/**
* @author Dang Duy Hieu
@@ -68,6 +71,8 @@
private static final String CRITERIA_SEP = ":";
private static final String REGEX_SEP = "|";
+ private static final String QUERY_VALUE_REGEX = "^[\\w\\s\\-]*$";
+
// -------------------------------------------------------------------------
// Properties
// -------------------------------------------------------------------------
@@ -130,11 +135,8 @@
String[] criteria = param.split( CRITERIA_SEP );
String filter = criteria[0];
String value = criteria[1];
-
- if ( StringUtils.isAlphanumericSpace( filter ) && StringUtils.isAlphanumericSpace( value ) )
- {
- map.put( filter, value );
- }
+
+ map.put( filter, value );
}
}
}
@@ -142,6 +144,52 @@
return map;
}
+ public static Set<String> getInvalidQueryParams( Set<String> params )
+ {
+ Set<String> invalid = new HashSet<>();
+
+ for ( String param : params )
+ {
+ if ( !isValidQueryParam( param ) )
+ {
+ invalid.add( param );
+ }
+ }
+
+ return invalid;
+ }
+
+ /**
+ * Indicates whether the given query parameter is valid.
+ */
+ public static final boolean isValidQueryParam( String param )
+ {
+ return StringUtils.isAlphanumeric( param );
+ }
+
+ public static Set<String> getInvalidQueryValues( Collection<String> values )
+ {
+ Set<String> invalid = new HashSet<>();
+
+ for ( String value : values )
+ {
+ if ( !isValidQueryValue( value ) )
+ {
+ invalid.add( value );
+ }
+ }
+
+ return invalid;
+ }
+
+ /**
+ * Indicates whether the given query value is valid.
+ */
+ public static final boolean isValidQueryValue( String value )
+ {
+ return value != null && value.matches( QUERY_VALUE_REGEX );
+ }
+
public static String getProtectedTablesRegex()
{
StringBuffer regex = new StringBuffer( "^.*?(\"|'|`|\\s|^)(" );
=== added directory 'dhis-2/dhis-api/src/test/java/org/hisp/dhis/sqlview'
=== added file 'dhis-2/dhis-api/src/test/java/org/hisp/dhis/sqlview/SqlViewTest.java'
--- dhis-2/dhis-api/src/test/java/org/hisp/dhis/sqlview/SqlViewTest.java 1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-api/src/test/java/org/hisp/dhis/sqlview/SqlViewTest.java 2015-09-15 09:43:39 +0000
@@ -0,0 +1,70 @@
+package org.hisp.dhis.sqlview;
+
+/*
+ * Copyright (c) 2004-2015, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * Neither the name of the HISP project nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
+
+/**
+ * @author Lars Helge Overland
+ */
+public class SqlViewTest
+{
+ @Test
+ public void testIsValidQueryValue()
+ {
+ assertTrue( SqlView.isValidQueryValue( "east" ) );
+ assertTrue( SqlView.isValidQueryValue( "NUMBER" ) );
+ assertTrue( SqlView.isValidQueryValue( "2015-03-01" ) );
+ assertTrue( SqlView.isValidQueryValue( "John Doe" ) );
+ assertTrue( SqlView.isValidQueryValue( "anc_1" ) );
+
+ assertFalse( SqlView.isValidQueryValue( "../var/dir" ) );
+ assertFalse( SqlView.isValidQueryValue( "delete from table;" ) );
+ }
+
+ @Test
+ public void testGetCriteria()
+ {
+ Set<String> params = Sets.newHashSet( "type:NUMBER", "aggregationType:AVERAGE" );
+
+ Map<String, String> expected = ImmutableMap.of( "type", "NUMBER", "aggregationType", "AVERAGE" );
+
+ assertEquals( expected, SqlView.getCriteria( params ) );
+ }
+}
=== modified file 'dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/sqlview/DefaultSqlViewService.java'
--- dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/sqlview/DefaultSqlViewService.java 2015-07-14 14:50:19 +0000
+++ dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/sqlview/DefaultSqlViewService.java 2015-09-15 09:43:39 +0000
@@ -34,7 +34,6 @@
import java.util.Set;
import java.util.regex.Matcher;
-import org.apache.commons.lang3.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hisp.dhis.common.Grid;
@@ -189,16 +188,16 @@
if ( variables != null )
{
- for ( String key : variables.keySet() )
+ for ( String param : variables.keySet() )
{
- if ( key != null && StringUtils.isAlphanumericSpace( key ) )
+ if ( param != null && SqlView.isValidQueryParam( param ) )
{
- final String regex = "\\$\\{(" + key + ")\\}";
- final String var = variables.get( key );
+ final String regex = "\\$\\{(" + param + ")\\}";
+ final String value = variables.get( param );
- if ( var != null && StringUtils.isAlphanumericSpace( var ) )
+ if ( value != null && SqlView.isValidQueryValue( value ) )
{
- sqlQuery = sqlQuery.replaceAll( regex, var );
+ sqlQuery = sqlQuery.replaceAll( regex, value );
}
}
}
@@ -255,12 +254,32 @@
{
violation = "Variables contains null value";
}
+
+ if ( variables != null && !SqlView.getInvalidQueryParams( variables.keySet() ).isEmpty() )
+ {
+ violation = "Variable params are invalid: " + SqlView.getInvalidQueryParams( variables.keySet() );
+ }
+ if ( variables != null && !SqlView.getInvalidQueryValues( variables.values() ).isEmpty() )
+ {
+ violation = "Variables are invalid: " + SqlView.getInvalidQueryValues( variables.values() );
+ }
+
if ( sqlView.isQuery() && !sqlVars.isEmpty() && ( variables == null || !variables.keySet().containsAll( sqlVars ) ) )
{
- violation = "SQL query contains variables which were not supplied in request: " + sqlVars;
+ violation = "SQL query contains variables which were not provided in request: " + sqlVars;
+ }
+
+ if ( criteria != null && !SqlView.getInvalidQueryParams( criteria.keySet() ).isEmpty() )
+ {
+ violation = "Criteria params are invalid: " + SqlView.getInvalidQueryParams( criteria.keySet() );
}
+ if ( criteria != null && !SqlView.getInvalidQueryValues( criteria.values() ).isEmpty() )
+ {
+ violation = "Criteria values are invalid: " + SqlView.getInvalidQueryValues( criteria.values() );
+ }
+
if ( sql.matches( SqlView.getProtectedTablesRegex() ) )
{
violation = "SQL query contains references to protected tables";
