Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape column names and paths in order by clause with backticks #38645

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Escape column names and paths in order by clause with backticks #38645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions ...src/test/java/io/quarkus/hibernate/orm/panache/deployment/test/JpaOperationsSortTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import io.quarkus.panache.common.Sort;
import io.quarkus.panache.common.exception.PanacheQueryException;
import io.quarkus.panache.hibernate.common.runtime.PanacheJpaUtil;

public class JpaOperationsSortTest {
Expand All @@ -18,7 +20,7 @@ public void testEmptySortByYieldsEmptyString() {
@Test
public void testSortBy() {
Sort sort = Sort.by("foo", "bar");
assertEquals(" ORDER BY foo , bar", PanacheJpaUtil.toOrderBy(sort));
assertEquals(" ORDER BY `foo` , `bar`", PanacheJpaUtil.toOrderBy(sort));
}

@Test
Expand All @@ -29,14 +31,48 @@ public void testEmptySortEmptyYieldsEmptyString() {

@Test
public void testSortByNullsFirst() {
Sort emptySort = Sort.by("foo", Sort.Direction.Ascending, Sort.NullPrecedence.NULLS_FIRST);
assertEquals(" ORDER BY foo NULLS FIRST", PanacheJpaUtil.toOrderBy(emptySort));
Sort sort = Sort.by("foo", Sort.Direction.Ascending, Sort.NullPrecedence.NULLS_FIRST);
assertEquals(" ORDER BY `foo` NULLS FIRST", PanacheJpaUtil.toOrderBy(sort));
}

@Test
public void testSortByNullsLast() {
Sort emptySort = Sort.by("foo", Sort.Direction.Descending, Sort.NullPrecedence.NULLS_LAST);
assertEquals(" ORDER BY foo DESC NULLS LAST", PanacheJpaUtil.toOrderBy(emptySort));
Sort sort = Sort.by("foo", Sort.Direction.Descending, Sort.NullPrecedence.NULLS_LAST);
assertEquals(" ORDER BY `foo` DESC NULLS LAST", PanacheJpaUtil.toOrderBy(sort));
}

@Test
public void testSortByColumnWithBacktick() {
Sort sort = Sort.by("jeanne", "d`arc");
Assertions.assertThrowsExactly(PanacheQueryException.class, () -> PanacheJpaUtil.toOrderBy(sort),
"Sort column name cannot have backticks");
}

@Test
public void testSortByQuotedColumn() {
Sort sort = Sort.by("`foo`", "bar");
assertEquals(" ORDER BY `foo` , `bar`", PanacheJpaUtil.toOrderBy(sort));
}

@Test
public void testSortByEmbeddedColumn() {
Sort sort = Sort.by("foo.bar");
assertEquals(" ORDER BY `foo`.`bar`", PanacheJpaUtil.toOrderBy(sort));
}

@Test
public void testSortByQuotedEmbeddedColumn() {
Sort sort1 = Sort.by("foo.`bar`");
assertEquals(" ORDER BY `foo`.`bar`", PanacheJpaUtil.toOrderBy(sort1));
Sort sort2 = Sort.by("`foo`.bar");
assertEquals(" ORDER BY `foo`.`bar`", PanacheJpaUtil.toOrderBy(sort2));
Sort sort3 = Sort.by("`foo`.`bar`");
assertEquals(" ORDER BY `foo`.`bar`", PanacheJpaUtil.toOrderBy(sort3));
}

@Test
public void testSortByDisabledEscaping() {
Sort sort1 = Sort.by("foo.`bar`").disableEscaping();
assertEquals(" ORDER BY foo.`bar`", PanacheJpaUtil.toOrderBy(sort1));
}
}
15 changes: 15 additions & 0 deletions extensions/panache/panache-common/runtime/src/main/java/io/quarkus/panache/common/Sort.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public void setNullPrecedence(NullPrecedence nullPrecedence) {
}

private List<Column> columns = new ArrayList<>();
private boolean escapingEnabled = true;

private Sort() {
}
Expand Down Expand Up @@ -293,6 +294,16 @@ public Sort and(String name, Direction direction, NullPrecedence nullPrecedence)
return this;
}

/**
* Disables escaping of column names with a backticks during HQL Order By clause generation
*
* @return this instance, modified.
*/
public Sort disableEscaping() {
escapingEnabled = false;
return this;
}

/**
* Get the sort columns
*
Expand All @@ -311,4 +322,8 @@ public List<Column> getColumns() {
public static Sort empty() {
return by();
}

public boolean isEscapingEnabled() {
return escapingEnabled;
}
}
32 changes: 31 additions & 1 deletion ...mon/runtime/src/main/java/io/quarkus/panache/hibernate/common/runtime/PanacheJpaUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ public static String toOrderBy(Sort sort) {
Sort.Column column = sort.getColumns().get(i);
if (i > 0)
sb.append(" , ");
sb.append(column.getName());
if (sort.isEscapingEnabled()) {
sb.append(escapeColumnName(column.getName()));
} else {
sb.append(column.getName());
}
if (column.getDirection() != Sort.Direction.Ascending) {
sb.append(" DESC");
}
Expand All @@ -191,4 +195,30 @@ public static String toOrderBy(Sort sort) {
}
return sb.toString();
}

private static StringBuilder escapeColumnName(String columnName) {
StringBuilder sb = new StringBuilder();
String[] path = columnName.split("\\.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have dots in property names? I assume not. I'm asking because if there's a way to escape dots like \. in column names, then we'd have to respect that, but I don't think column names support dot escaping.

I just cheked the HQL grammar and there are no escaped dots, so we're good.

for (int j = 0; j < path.length; j++) {
if (j > 0)
sb.append('.');
sb.append('`').append(unquoteColumnName(path[j])).append('`');
}
return sb;
}

private static String unquoteColumnName(String columnName) {
String unquotedColumnName;
//Note HQL uses backticks to escape/quote special words that are used as identifiers
if (columnName.charAt(0) == '`' && columnName.charAt(columnName.length() - 1) == '`') {
unquotedColumnName = columnName.substring(1, columnName.length() - 1);
} else {
unquotedColumnName = columnName;
}
// Note we're not dealing with columns but with entity attributes so no backticks expected in unquoted column name
if (unquotedColumnName.indexOf('`') >= 0) {
throw new PanacheQueryException("Sort column name cannot have backticks");
}
return unquotedColumnName;
}
}
28 changes: 28 additions & 0 deletions ...gration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1741,6 +1741,34 @@ public String testSortByNullPrecedence() {
return "OK";
}

@GET
@Path("testSortByEmbedded")
@Transactional
public String testSortByEmbedded() {
Person.deleteAll();

Person stefPerson = new Person();
stefPerson.name = "Stef";
stefPerson.description = new PersonDescription();
stefPerson.description.size = 0;
stefPerson.persist();

Person josePerson = new Person();
josePerson.name = "Jose";
josePerson.description = new PersonDescription();
josePerson.description.size = 100;
josePerson.persist();

List<Person> persons = Person.findAll(Sort.by("description.size", Sort.Direction.Descending)).list();
assertEquals(josePerson.id, persons.get(0).id);
persons = Person.findAll(Sort.by("description.size", Sort.Direction.Ascending)).list();
assertEquals(josePerson.id, persons.get(persons.size() - 1).id);

Person.deleteAll();

return "OK";
}

@GET
@Path("testEnhancement27184DeleteDetached")
// NOT @Transactional
Expand Down
5 changes: 5 additions & 0 deletions ...s/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ public void testSortByNullPrecedence() {
RestAssured.when().get("/test/testSortByNullPrecedence").then().body(is("OK"));
}

@Test
public void testSortByEmbedded() {
RestAssured.when().get("/test/testSortByEmbedded").then().body(is("OK"));
}

@Test
public void testJaxbAnnotationTransfer() {
RestAssured.when()
Expand Down
Loading