From 567ea3241b25c52c60eb556d7d79febb471ebef7 Mon Sep 17 00:00:00 2001 From: Vijay Nayar Date: Wed, 6 Nov 2024 17:39:35 +0100 Subject: [PATCH 1/3] Bug: DelayedLoadRelations uses inconsistent quoting for string-literals. If using HQL on an Entity that is implicitly joined to other Entities via @OneToOne, @OneToMany, @ManyToOne, or @ManyToMany, and an ID value has a single-quote (') in it, the HQL-escaping ('') is not unescaped, and it gets escaped into the dialect, resulting in (\\'\\'), and a query that fails to join. Example debug statements: === [trace] session.d:1207:delayedLoadRelations unknownKeys = [Bucky O'Hare] [trace] session.d:1208:delayedLoadRelations createCommaSeparatedKeyList(unknownKeys) = 'Bucky O''Hare' extra quote is being added here ^ [trace] session.d:1290:listObjects SQL: SELECT _t1.contract_code, ... FROM finance_contract_refs AS _t1 WHERE _t1.contract_code IN ( 'Bucky O\\'\\'Hare') === The fix is to modify the HQL Parser to unescape ('') as ('), and then let specific dialects re-escape the ('). PostgreSQL, by default, does not support C-style escape characters unless quoted as E'hello\nthere\n' (note the leading (E)). However, all dialects support the SQL standard ('') syntax. --- hdtest/source/generaltest.d | 13 +++++++++++++ source/hibernated/annotations.d | 1 - source/hibernated/dialect.d | 3 ++- source/hibernated/query.d | 9 +++++++-- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/hdtest/source/generaltest.d b/hdtest/source/generaltest.d index d8d79d2..e88e3e8 100644 --- a/hdtest/source/generaltest.d +++ b/hdtest/source/generaltest.d @@ -253,5 +253,18 @@ class GeneralTest : HibernateTest { assert(allUsers.length == 2); // Should only be 2 users now } } + + @Test("quote escape test") + void quoteEscapeTest() { + Session sess = sessionFactory.openSession(); + scope(exit) sess.close(); + + auto a1 = new Asset(); + a1.name = "Bucky O'Hare"; + int id = sess.save(a1).get!int; + + auto query = sess.createQuery("FROM Asset WHERE name=:Name").setParameter("Name", "Bucky O'Hare"); + writeln("FLOOB: query results", query.listRows()); + } } diff --git a/source/hibernated/annotations.d b/source/hibernated/annotations.d index 79231b0..b9c49ea 100644 --- a/source/hibernated/annotations.d +++ b/source/hibernated/annotations.d @@ -266,7 +266,6 @@ struct ManyToMany { immutable string joinColumn2; } - unittest { @Entity @Table("user") diff --git a/source/hibernated/dialect.d b/source/hibernated/dialect.d index 94e2225..b6d7168 100755 --- a/source/hibernated/dialect.d +++ b/source/hibernated/dialect.d @@ -109,7 +109,8 @@ abstract class Dialect { string res = "'"; foreach(ch; s) { switch(ch) { - case '\'': res ~= "\\\'"; break; + // All dialects support the SQL-standard way of escaping a (') character via (''). + case '\'': res ~= "''"; break; case '\"': res ~= "\\\""; break; case '\\': res ~= "\\\\"; break; case '\0': res ~= "\\n"; break; diff --git a/source/hibernated/query.d b/source/hibernated/query.d index 8d5aa33..0a69ae2 100644 --- a/source/hibernated/query.d +++ b/source/hibernated/query.d @@ -1443,6 +1443,12 @@ class Token { } +/** + * Converts an HQL string into a series of tokens to be later converted to SQL. + * + * HQL follows the Jakarta Persistence specification, which requires escaping (') characters via (''). + * See: https://jakarta.ee/specifications/persistence/3.2/jakarta-persistence-spec-3.2#literals + */ Token[] tokenize(string s) { Token[] res; int startpos = 0; @@ -1521,11 +1527,10 @@ Token[] tokenize(string s) { // string constant i++; for(int j=i; j Date: Wed, 6 Nov 2024 18:04:26 +0100 Subject: [PATCH 2/3] Remove debug output. --- hdtest/source/generaltest.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hdtest/source/generaltest.d b/hdtest/source/generaltest.d index e88e3e8..a80f98d 100644 --- a/hdtest/source/generaltest.d +++ b/hdtest/source/generaltest.d @@ -263,8 +263,8 @@ class GeneralTest : HibernateTest { a1.name = "Bucky O'Hare"; int id = sess.save(a1).get!int; - auto query = sess.createQuery("FROM Asset WHERE name=:Name").setParameter("Name", "Bucky O'Hare"); - writeln("FLOOB: query results", query.listRows()); + auto query = sess.createQuery("FROM Asset WHERE name=:Name").setParameter("Name", "Bucky O'Hare").list!Asset; + assert(query.length == 1); } } From 47008eec5d29aac28ca913260f1e08779d3d35c7 Mon Sep 17 00:00:00 2001 From: Vijay Nayar Date: Thu, 7 Nov 2024 09:24:47 +0100 Subject: [PATCH 3/3] Add another quoting test with the string literal directly in the query rather than a parameter. --- hdtest/source/generaltest.d | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/hdtest/source/generaltest.d b/hdtest/source/generaltest.d index a80f98d..dd46d98 100644 --- a/hdtest/source/generaltest.d +++ b/hdtest/source/generaltest.d @@ -263,8 +263,14 @@ class GeneralTest : HibernateTest { a1.name = "Bucky O'Hare"; int id = sess.save(a1).get!int; - auto query = sess.createQuery("FROM Asset WHERE name=:Name").setParameter("Name", "Bucky O'Hare").list!Asset; - assert(query.length == 1); + auto result = sess.createQuery("FROM Asset WHERE name=:Name") + .setParameter("Name", "Bucky O'Hare") + .list!Asset(); + assert(result.length == 1); + + result = sess.createQuery("FROM Asset WHERE name='Bucky O''Hare'") + .list!Asset(); + assert(result.length == 1); } }