DATAJPA-965 - Fix potential blind SQL injection in Sort when used in combination with @Query.

We now decline sort expressions that contain functions such as ORDER BY LENGTH(name) when used with repository having a String query defined via the @Query annotation. Think of a query method as follows: @Query("select p from Person p where LOWER(p.lastname) = LOWER(:lastname)") List<Person> findByLastname(@Param("lastname") String lastname, Sort sort); Calls to findByLastname("lannister", new Sort("LENGTH(firstname)")) from now on throw an Exception indicating function calls are not allowed within the _ORDER BY_ clause. However you still can use JpaSort.unsafe("LENGTH(firstname)") to restore the behavior. Kudos to Niklas Särökaari, Joona Immonen, Arto Santala, Antti Virtanen, Michael Holopainen and Antti Ahola who brought this to our attention.
2016-09-06 13:34:08 +02:00
parent 890fd7f15d
commit edd497b9c9
5 changed files with 491 additions and 6 deletions
--- a/src/main/asciidoc/jpa.adoc
+++ b/src/main/asciidoc/jpa.adoc
@@ -303,6 +303,42 @@ public interface UserRepository extends JpaRepository<User, Long> {

 This also works with named native queries by adding the suffix `.count` to a copy of your query. Be aware that you probably must register a result set mapping for your count query, though.

+[[jpa.query-methods.sorting]]
+=== Using Sort
+
+Sorting can be done be either providing a `PageRequest` or using `Sort` directly. The properties actually used within the `Order` instances of `Sort` need to match to your domain model, which means they need to resolve to either a property or an alias used within the query. The JPQL defines this as a _state_field_path_expression_.
+
+[NOTE]
+====
+Using any non referenceable path expression leads to an Exception.
+====
+
+Using `Sort` together with <<jpa.query-methods.at-query, @Query>> however allows you to sneak in non path checked `Order` instances containing _functions_ within the `ORDER BY` clause. This is possible because the `Order` is just appended to the given query string. By default we will reject any `Order` instance containing function calls, but you can use `JpaSort.unsafe` to add potentially unsafe ordering.
+
+.Using Sort and JpaSort
+====
+[source, java]
+----
+public interface UserRepository extends JpaRepository<User, Long> {
+
+  @Query("select u from User u where u.lastname like ?1%")
+  List<User> findByAndSort(String lastname, Sort sort);
+
+  @Query("select u.id, LENGTH(u.firstname) as fn_len from User u where u.lastname like ?1%")
+  List<Object[]> findByAsArrayAndSort(String lastname, Sort sort);
+}
+
+repo.findByAndSort("lannister", new Sort("firstname"));               <1>
+repo.findByAndSort("stark", new Sort("LENGTH(firstname)"));           <2>
+repo.findByAndSort("targaryen", JpaSort.unsafe("LENGTH(firstname)")); <3>
+repo.findByAsArrayAndSort("bolton", new Sort("fn_len"));              <4>
+----
+<1> Valid `Sort` expression pointing to property in domain model.
+<2> Invalid `Sort` containing function call. Thows Exception.
+<3> Valid `Sort` containing explicitly _unsafe_ `Order`.
+<4> Valid `Sort` expression pointing to aliased function.
+====
+
 [[jpa.named-parameters]]
 === Using named parameters

--- a/src/main/java/org/springframework/data/jpa/domain/JpaSort.java
+++ b/src/main/java/org/springframework/data/jpa/domain/JpaSort.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2016 the original author or authors.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -32,6 +32,7 @@ import org.springframework.util.Assert;
 * 
 * @author Thomas Darimont
 * @author Oliver Gierke
+ * @author Christoph Strobl
 */
 public class JpaSort extends Sort {

@@ -83,6 +84,10 @@ public class JpaSort extends Sort {
 		super(combine(orders, direction, paths));
 	}

+	private JpaSort(List<Order> orders) {
+		super(orders);
+	}
+
 	/**
 	 * Returns a new {@link JpaSort} with the given sorting criteria added to the current one.
 	 * 
@@ -117,6 +122,30 @@ public class JpaSort extends Sort {
 		return new JpaSort(existing, direction, Arrays.asList(paths));
 	}

+	/**
+	 * Returns a new {@link JpaSort} with the given sorting criteria added to the current one.
+	 *
+	 * @param direction can be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public JpaSort andUnsafe(Direction direction, String... properties) {
+
+		Assert.notEmpty(properties, "Properties must not be null!");
+
+		List<Order> orders = new ArrayList<Order>();
+
+		for (Order order : this) {
+			orders.add(order);
+		}
+
+		for (String property : properties) {
+			orders.add(new JpaOrder(direction, property));
+		}
+
+		return new JpaSort(orders, direction, Collections.<Path<?, ?>> emptyList());
+	}
+
 	/**
 	 * Turns the given {@link Attribute}s into {@link Path}s.
 	 * 
@@ -174,6 +203,51 @@ public class JpaSort extends Sort {
 		return new Path<T, S>(Arrays.asList(attribute));
 	}

+	/**
+	 * Creates new unsafe {@link JpaSort} based on given properties.
+	 *
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public static JpaSort unsafe(String... properties) {
+		return unsafe(Sort.DEFAULT_DIRECTION, properties);
+	}
+
+	/**
+	 * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties.
+	 *
+	 * @param direction must not be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public static JpaSort unsafe(Direction direction, String... properties) {
+
+		Assert.notNull(direction, "Direction must not be null!");
+		Assert.notEmpty(properties, "Properties must not be empty!");
+		Assert.noNullElements(properties, "Properties must not contain null values!");
+
+		return unsafe(direction, Arrays.asList(properties));
+	}
+
+	/**
+	 * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties.
+	 *
+	 * @param direction must not be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public static JpaSort unsafe(Direction direction, List<String> properties) {
+
+		Assert.notEmpty(properties, "Properties must not be empty!");
+
+		List<Order> orders = new ArrayList<Order>();
+		for (String property : properties) {
+			orders.add(new JpaOrder(direction, property));
+		}
+
+		return new JpaSort(orders);
+	}
+
 	/**
 	 * Value object to abstract a collection of {@link Attribute}s.
 	 * 
@@ -233,4 +307,131 @@ public class JpaSort extends Sort {
 			return builder.length() == 0 ? "" : builder.substring(0, builder.lastIndexOf("."));
 		}
 	}
+
+	/**
+	 * @author Christoph Strobl
+	 */
+	public static class JpaOrder extends Order {
+
+		private final boolean unsafe;
+		private final boolean ignoreCase;
+
+		/**
+		 * Creates a new {@link JpaOrder} instance. if order is {@literal null} then order defaults to
+		 * {@link Sort#DEFAULT_DIRECTION}
+		 *
+		 * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}.
+		 * @param property must not be {@literal null}.
+		 */
+		private JpaOrder(Direction direction, String property) {
+			this(direction, property, NullHandling.NATIVE);
+		}
+
+		/**
+		 * Creates a new {@link Order} instance. if order is {@literal null} then order defaults to
+		 * {@link Sort#DEFAULT_DIRECTION}.
+		 *
+		 * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}.
+		 * @param property must not be {@literal null}.
+		 * @param nullHandlingHint can be {@literal null}, will default to {@link NullHandling#NATIVE}.
+		 */
+		private JpaOrder(Direction direction, String property, NullHandling nullHandlingHint) {
+			this(direction, property, nullHandlingHint, false, true);
+		}
+
+		private JpaOrder(Direction direction, String property, NullHandling nullHandling, boolean ignoreCase,
+				boolean unsafe) {
+
+			super(direction, property, nullHandling);
+			this.ignoreCase = ignoreCase;
+			this.unsafe = unsafe;
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.Direction)
+		 */
+		@Override
+		public JpaOrder with(Direction order) {
+			return new JpaOrder(order, getProperty(), getNullHandling(), isIgnoreCase(), this.unsafe);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.NullHandling)
+		 */
+		@Override
+		public JpaOrder with(NullHandling nullHandling) {
+			return new JpaOrder(getDirection(), getProperty(), nullHandling, isIgnoreCase(), this.unsafe);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#nullsFirst()
+		 */
+		@Override
+		public JpaOrder nullsFirst() {
+			return with(NullHandling.NULLS_FIRST);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#nullsLast()
+		 */
+		@Override
+		public JpaOrder nullsLast() {
+			return with(NullHandling.NULLS_LAST);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#nullsNative()
+		 */
+		public JpaOrder nullsNative() {
+			return with(NullHandling.NATIVE);
+		}
+
+		/**
+		 * Creates new {@link Sort} with potentially unsafe {@link Order} instances.
+		 *
+		 * @param properties must not be {@literal null}.
+		 * @return
+		 */
+		public Sort withUnsafe(String... properties) {
+
+			Assert.notEmpty(properties, "Properties must not be empty!");
+			Assert.noNullElements(properties, "Properties must not contain null values!");
+
+			List<Order> orders = new ArrayList<Order>();
+			for (String property : properties) {
+				orders.add(new JpaOrder(getDirection(), property, getNullHandling(), isIgnoreCase(), this.unsafe));
+			}
+			return new Sort(orders);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#ignoreCase()
+		 */
+		@Override
+		public JpaOrder ignoreCase() {
+			return new JpaOrder(getDirection(), getProperty(), getNullHandling(), true, this.unsafe);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#isIgnoreCase()
+		 */
+		@Override
+		public boolean isIgnoreCase() {
+			return super.isIgnoreCase() || ignoreCase;
+		}
+
+		/**
+		 * @return true if {@link JpaOrder} created {@link #withUnsafe(String...)}.
+		 */
+		public boolean isUnsafe() {
+			return unsafe;
+		}
+	}
 }
--- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
+++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
@@ -53,8 +53,10 @@ import javax.persistence.metamodel.ManagedType;
 import javax.persistence.metamodel.PluralAttribute;

 import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.jpa.domain.JpaSort.JpaOrder;
 import org.springframework.data.mapping.PropertyPath;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -66,6 +68,7 @@ import org.springframework.util.StringUtils;
 * @author Kevin Raymond
 * @author Thomas Darimont
 * @author Komi Innocent
+ * @author Christoph Strobl
 */
 public abstract class QueryUtils {

@@ -102,6 +105,10 @@ public abstract class QueryUtils {
 	private static final int QUERY_JOIN_ALIAS_GROUP_INDEX = 2;
 	private static final int VARIABLE_NAME_GROUP_INDEX = 4;

+	private static final Pattern PUNCTATION_PATTERN = Pattern.compile(".*((?![\\._])[\\p{Punct}|\\s])");
+	private static final String FUNCTION_ALIAS_GROUP_NAME = "alias";
+	private static final Pattern FUNCTION_PATTERN;
+
 	static {

 		StringBuilder builder = new StringBuilder();
@@ -145,6 +152,13 @@ public abstract class QueryUtils {
 		builder.append("\\)");

 		CONSTRUCTOR_EXPRESSION = compile(builder.toString(), CASE_INSENSITIVE + DOTALL);
+
+		builder = new StringBuilder();
+		builder.append("\\s+"); // at least one space
+		builder.append("\\w+\\([0-9a-zA-z\\._,\\s']+\\)"); // any function call including parameters within the brackets
+		builder.append("\\s+[as|AS]+\\s+(?<" + FUNCTION_ALIAS_GROUP_NAME + ">[\\w\\.]+)"); // the potential alias
+
+		FUNCTION_PATTERN = compile(builder.toString());
 	}

 	/**
@@ -227,9 +241,10 @@ public abstract class QueryUtils {
 		}

 		Set<String> aliases = getOuterJoinAliases(query);
+		Set<String> functionAliases = getFunctionAliases(query);

 		for (Order order : sort) {
-			builder.append(getOrderClause(aliases, alias, order)).append(", ");
+			builder.append(getOrderClause(aliases, functionAliases, alias, order)).append(", ");
 		}

 		builder.delete(builder.length() - 2, builder.length());
@@ -246,9 +261,16 @@ public abstract class QueryUtils {
 	 * @param order the order object to build the clause for.
 	 * @return
 	 */
-	private static String getOrderClause(Set<String> joinAliases, String alias, Order order) {
+	private static String getOrderClause(Set<String> joinAliases, Set<String> functionAlias, String alias, Order order) {

 		String property = order.getProperty();
+
+		checkSortExpression(order);
+
+		if (functionAlias.contains(property)) {
+			return String.format("%s %s", property, toJpaDirection(order));
+		}
+
 		boolean qualifyReference = !property.contains("("); // ( indicates a function

 		for (String joinAlias : joinAliases) {
@@ -287,6 +309,28 @@ public abstract class QueryUtils {
 		return result;
 	}

+	/**
+	 * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}.
+	 *
+	 * @param query
+	 * @return
+	 */
+	static Set<String> getFunctionAliases(String query) {
+
+		Set<String> result = new HashSet<String>();
+		Matcher matcher = FUNCTION_PATTERN.matcher(query);
+
+		while (matcher.find()) {
+
+			String alias = matcher.group(FUNCTION_ALIAS_GROUP_NAME);
+			if (StringUtils.hasText(alias)) {
+				result.add(alias);
+			}
+		}
+
+		return result;
+	}
+
 	private static String toJpaDirection(Order order) {
 		return order.getDirection().name().toLowerCase(Locale.US);
 	}
@@ -617,4 +661,23 @@ public abstract class QueryUtils {

 		return false;
 	}
+
+	/**
+	 * Check any given {@link JpaOrder#isUnsafe()} order for presence of at least one property offending the
+	 * {@link #PUNCTATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
+	 *
+	 * @param order
+	 */
+	private static void checkSortExpression(Order order) {
+
+		if (order instanceof JpaOrder && ((JpaOrder) order).isUnsafe()) {
+			return;
+		}
+
+		if (PUNCTATION_PATTERN.matcher(order.getProperty()).find()) {
+			throw new InvalidDataAccessApiUsageException(String
+					.format("Sort expression '%s' must not contain functions or expressions. Please use JpaSort.unsafe.", order));
+		}
+	}
+
 }
--- a/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java
+++ b/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2016 the original author or authors.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@ import org.junit.runner.RunWith;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.jpa.domain.JpaSort.JpaOrder;
 import org.springframework.data.jpa.domain.JpaSort.Path;
 import org.springframework.data.jpa.domain.sample.Address_;
 import org.springframework.data.jpa.domain.sample.MailMessage_;
@@ -47,6 +48,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 * @see DATAJPA-12
 * @author Thomas Darimont
 * @author Oliver Gierke
+ * @author Christoph Strobl
 */
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration("classpath:infrastructure.xml")
@@ -173,4 +175,56 @@ public class JpaSortTests {
 		assertThat(new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)), //
 				hasItem(new Order(ASC, "colleagues.roles.name")));
 	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void createsUnsafeSortCorrectly() {
+
+		JpaSort sort = JpaSort.unsafe(DESC, "foo.bar");
+
+		assertThat(sort, hasItem(new Order(DESC, "foo.bar")));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void createsUnsafeSortWithMultiplePropertiesCorrectly() {
+
+		JpaSort sort = JpaSort.unsafe(DESC, "foo.bar", "spring.data");
+
+		assertThat(sort, hasItems(new Order(DESC, "foo.bar"), new Order(DESC, "spring.data")));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+		assertThat(sort.getOrderFor("spring.data"), is(instanceOf(JpaOrder.class)));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void combinesSafeAndUnsafeSortCorrectly() {
+
+		JpaSort sort = new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)).andUnsafe(DESC, "foo.bar");
+
+		assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar")));
+		assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class))));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void combinesUnsafeAndSafeSortCorrectly() {
+
+		Sort sort = JpaSort.unsafe(DESC, "foo.bar").and(ASC, path(User_.colleagues).dot(User_.roles).dot(Role_.name));
+
+		assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar")));
+		assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class))));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+	}
+
 }
--- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java
+++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2015 the original author or authors.
+ * Copyright 2008-2016 the original author or authors.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -23,7 +23,9 @@ import java.util.Set;

 import org.hamcrest.Matcher;
 import org.junit.Test;
+import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Sort;
+import org.springframework.data.jpa.domain.JpaSort;

 /**
 * Unit test for {@link QueryUtils}.
@@ -31,6 +33,7 @@ import org.springframework.data.domain.Sort;
 * @author Oliver Gierke
 * @author Thomas Darimont
 * @author Komi Innocent
+ * @author Christoph Strobl
 */
 public class QueryUtilsUnitTests {

@@ -230,7 +233,7 @@ public class QueryUtilsUnitTests {
 	/**
 	 * @see DATAJPA-148
 	 */
-	@Test
+	@Test(expected = InvalidDataAccessApiUsageException.class)
 	public void doesNotPrefixSortsIfFunction() {

 		Sort sort = new Sort("sum(foo)");
@@ -361,6 +364,134 @@ public class QueryUtilsUnitTests {
 				endsWith("order by firstname asc"));
 	}

+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test(expected = InvalidDataAccessApiUsageException.class)
+	public void doesNotAllowWhitespaceInSort() {
+
+		Sort sort = new Sort("case when foo then bar");
+		applySorting("select p from Person p", sort, "p");
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixUnsageJpaSortFunctionCalls() {
+
+		JpaSort sort = JpaSort.unsafe("sum(foo)");
+		assertThat(applySorting("select p from Person p", sort, "p"), endsWith("order by sum(foo) asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixMultipleAliasedFunctionCalls() {
+
+		String query = "SELECT AVG(m.price) AS avgPrice, SUM(m.stocks) AS sumStocks FROM Magazine m";
+		Sort sort = new Sort("avgPrice", "sumStocks");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc, sumStocks asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixSingleAliasedFunctionCalls() {
+
+		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
+		Sort sort = new Sort("avgPrice");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void prefixesSingleNonAliasedFunctionCallRelatedSortProperty() {
+
+		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
+		Sort sort = new Sort("someOtherProperty");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by m.someOtherProperty asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void prefixesNonAliasedFunctionCallRelatedSortPropertyWhenSelectClauseContainesAliasedFunctionForDifferentProperty() {
+
+		String query = "SELECT m.name, AVG(m.price) AS avgPrice FROM Magazine m";
+		Sort sort = new Sort("name", "avgPrice");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by m.name asc, avgPrice asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithMultipleNumericParameters() {
+
+		String query = "SELECT SUBSTRING(m.name, 2, 5) AS trimmedName FROM Magazine m";
+		Sort sort = new Sort("trimmedName");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by trimmedName asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithMultipleStringParameters() {
+
+		String query = "SELECT CONCAT(m.name, 'foo') AS extendedName FROM Magazine m";
+		Sort sort = new Sort("extendedName");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by extendedName asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithUnderscores() {
+
+		String query = "SELECT AVG(m.price) AS avg_price FROM Magazine m";
+		Sort sort = new Sort("avg_price");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avg_price asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithDots() {
+
+		String query = "SELECT AVG(m.price) AS m.avg FROM Magazine m";
+		Sort sort = new Sort("m.avg");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by m.avg asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWhenQueryStringContainsMultipleWhiteSpaces() {
+
+		String query = "SELECT  AVG(  m.price  )   AS   avgPrice   FROM Magazine   m";
+		Sort sort = new Sort("avgPrice");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc"));
+	}
+
 	private static void assertCountQuery(String originalQuery, String countQuery) {
 		assertThat(createCountQueryFor(originalQuery), is(countQuery));
 	}