Skip to content

Commit f70ef04

Browse files
Kevin GallardoAlexandre Dutra
Kevin Gallardo
authored and
Alexandre Dutra
committed
Prevent QueryBuilder.quote() from applying duplicate double quotes (JAVA-712).
2 parents 2e811de + 7386ca6 commit f70ef04

2 files changed

Lines changed: 90 additions & 54 deletions

File tree

‎driver-core/src/main/java/com/datastax/driver/core/querybuilder/QueryBuilder.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,7 @@ public static Truncate truncate(TableMetadata table) {
226226
* @return the quoted column name.
227227
*/
228228
public static String quote(String columnName) {
229-
StringBuilder sb = new StringBuilder();
230-
sb.append('"');
231-
Utils.appendName(columnName, sb);
232-
sb.append('"');
233-
return sb.toString();
229+
return '"' + columnName + '"';
234230
}
235231

236232
/**

‎driver-core/src/test/java/com/datastax/driver/core/querybuilder/QueryBuilderTest.java‎

Lines changed: 89 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,18 @@
1818
import java.math.BigDecimal;
1919
import java.math.BigInteger;
2020
import java.net.InetAddress;
21-
import java.nio.ByteBuffer;
2221
import java.util.*;
2322

2423
import com.google.common.collect.ImmutableList;
2524
import com.google.common.collect.ImmutableMap;
2625
import org.testng.annotations.Test;
2726

2827
import static org.assertj.core.api.Assertions.assertThat;
29-
import static org.testng.Assert.*;
28+
import static org.testng.Assert.assertEquals;
29+
import static org.testng.Assert.assertTrue;
30+
import static org.testng.Assert.fail;
3031

3132
import com.datastax.driver.core.*;
32-
import com.datastax.driver.core.querybuilder.Delete.Where;
3333
import com.datastax.driver.core.utils.Bytes;
3434
import com.datastax.driver.core.utils.CassandraVersion;
3535

@@ -54,10 +54,10 @@ public void selectTest() throws Exception {
5454

5555
query = "SELECT a,b,\"C\" FROM foo WHERE a IN ('127.0.0.1','127.0.0.3') AND \"C\"='foo' ORDER BY a ASC,b DESC LIMIT 42;";
5656
select = select("a", "b", quote("C")).from("foo")
57-
.where(in("a", InetAddress.getByName("127.0.0.1"), InetAddress.getByName("127.0.0.3")))
58-
.and(eq(quote("C"), "foo"))
59-
.orderBy(asc("a"), desc("b"))
60-
.limit(42);
57+
.where(in("a", InetAddress.getByName("127.0.0.1"), InetAddress.getByName("127.0.0.3")))
58+
.and(eq(quote("C"), "foo"))
59+
.orderBy(asc("a"), desc("b"))
60+
.limit(42);
6161
assertEquals(select.toString(), query);
6262

6363
query = "SELECT writetime(a),ttl(a) FROM foo ALLOW FILTERING;";
@@ -123,35 +123,35 @@ public void selectTest() throws Exception {
123123
assertEquals(select.toString(), query);
124124

125125
try {
126-
select = select().countAll().from("foo").orderBy(asc("a"), desc("b")).orderBy(asc("a"), desc("b"));
126+
select().countAll().from("foo").orderBy(asc("a"), desc("b")).orderBy(asc("a"), desc("b"));
127127
fail();
128128
} catch (IllegalStateException e) {
129129
assertEquals(e.getMessage(), "An ORDER BY clause has already been provided");
130130
}
131131

132132
try {
133-
select = select().column("a").all().from("foo");
133+
select().column("a").all().from("foo");
134134
fail();
135135
} catch (IllegalStateException e) {
136136
assertEquals(e.getMessage(), "Some columns ([a]) have already been selected.");
137137
}
138138

139139
try {
140-
select = select().column("a").countAll().from("foo");
140+
select().column("a").countAll().from("foo");
141141
fail();
142142
} catch (IllegalStateException e) {
143143
assertEquals(e.getMessage(), "Some columns ([a]) have already been selected.");
144144
}
145145

146146
try {
147-
select = select().all().from("foo").limit(-42);
147+
select().all().from("foo").limit(-42);
148148
fail();
149149
} catch (IllegalArgumentException e) {
150150
assertEquals(e.getMessage(), "Invalid LIMIT value, must be strictly positive");
151151
}
152152

153153
try {
154-
select = select().all().from("foo").limit(42).limit(42);
154+
select().all().from("foo").limit(42).limit(42);
155155
fail();
156156
} catch (IllegalStateException e) {
157157
assertEquals(e.getMessage(), "A LIMIT value has already been provided");
@@ -167,48 +167,67 @@ public void insertTest() throws Exception {
167167

168168
query = "INSERT INTO foo(a,b,\"C\",d) VALUES (123,'127.0.0.1','foo''bar',{'x':3,'y':2}) USING TIMESTAMP 42 AND TTL 24;";
169169
insert = insertInto("foo")
170-
.value("a", 123)
171-
.value("b", InetAddress.getByName("127.0.0.1"))
172-
.value(quote("C"), "foo'bar")
173-
.value("d", new TreeMap<String, Integer>(){{ put("x", 3); put("y", 2); }})
174-
.using(timestamp(42)).and(ttl(24));
170+
.value("a", 123)
171+
.value("b", InetAddress.getByName("127.0.0.1"))
172+
.value(quote("C"), "foo'bar")
173+
.value("d", new TreeMap<String, Integer>() {{
174+
put("x", 3);
175+
put("y", 2);
176+
}})
177+
.using(timestamp(42)).and(ttl(24));
175178
assertEquals(insert.toString(), query);
176179

177180
query = "INSERT INTO foo(a,b) VALUES (2,null);";
178181
insert = insertInto("foo")
179-
.value("a", 2)
180-
.value("b", null);
182+
.value("a", 2)
183+
.value("b", null);
181184
assertEquals(insert.toString(), query);
182185

183186
query = "INSERT INTO foo(a,b) VALUES ({2,3,4},3.4) USING TTL 24 AND TIMESTAMP 42;";
184-
insert = insertInto("foo").values(new String[]{ "a", "b"}, new Object[]{ new TreeSet<Integer>(){{ add(2); add(3); add(4); }}, 3.4 }).using(ttl(24)).and(timestamp(42));
187+
insert = insertInto("foo").values(new String[]{ "a", "b" }, new Object[]{ new TreeSet<Integer>() {{
188+
add(2);
189+
add(3);
190+
add(4);
191+
}}, 3.4 }).using(ttl(24)).and(timestamp(42));
185192
assertEquals(insert.toString(), query);
186193

187194
query = "INSERT INTO foo.bar(a,b) VALUES ({2,3,4},3.4) USING TTL ? AND TIMESTAMP ?;";
188195
insert = insertInto("foo", "bar")
189-
.values(new String[]{ "a", "b"}, new Object[]{ new TreeSet<Integer>(){{ add(2); add(3); add(4); }}, 3.4 })
190-
.using(ttl(bindMarker()))
191-
.and(timestamp(bindMarker()));
196+
.values(new String[]{ "a", "b" }, new Object[]{ new TreeSet<Integer>() {{
197+
add(2);
198+
add(3);
199+
add(4);
200+
}}, 3.4 })
201+
.using(ttl(bindMarker()))
202+
.and(timestamp(bindMarker()));
192203
assertEquals(insert.toString(), query);
193204

194205
// commutative result of TIMESTAMP
195206
query = "INSERT INTO foo.bar(a,b,c) VALUES ({2,3,4},3.4,123) USING TIMESTAMP 42;";
196207
insert = insertInto("foo", "bar")
197-
.using(timestamp(42))
198-
.values(new String[]{ "a", "b"}, new Object[]{ new TreeSet<Integer>(){{ add(2); add(3); add(4); }}, 3.4 })
199-
.value("c", 123);
208+
.using(timestamp(42))
209+
.values(new String[]{ "a", "b" }, new Object[]{ new TreeSet<Integer>() {{
210+
add(2);
211+
add(3);
212+
add(4);
213+
}}, 3.4 })
214+
.value("c", 123);
200215
assertEquals(insert.toString(), query);
201216

202217
// commutative result of value() and values()
203218
query = "INSERT INTO foo(c,a,b) VALUES (123,{2,3,4},3.4) USING TIMESTAMP 42;";
204219
insert = insertInto("foo")
205-
.using(timestamp(42))
206-
.value("c", 123)
207-
.values(new String[]{ "a", "b"}, new Object[]{ new TreeSet<Integer>(){{ add(2); add(3); add(4); }}, 3.4 });
220+
.using(timestamp(42))
221+
.value("c", 123)
222+
.values(new String[]{ "a", "b" }, new Object[]{ new TreeSet<Integer>() {{
223+
add(2);
224+
add(3);
225+
add(4);
226+
}}, 3.4 });
208227
assertEquals(insert.toString(), query);
209228

210229
try {
211-
insert = insertInto("foo").values(new String[]{ "a", "b"}, new Object[]{ 1, 2, 3 });
230+
insertInto("foo").values(new String[]{ "a", "b" }, new Object[]{ 1, 2, 3 });
212231
fail();
213232
} catch (IllegalArgumentException e) {
214233
assertEquals(e.getMessage(), "Got 2 names but 3 values");
@@ -250,20 +269,31 @@ public void updateTest() throws Exception {
250269
assertEquals(update.toString(), query);
251270

252271
query = "UPDATE foo SET b=b-[1,2,3],c=c+{1},d=d+{2,3,4};";
253-
update = update("foo").with(discardAll("b", Arrays.asList(1, 2, 3))).and(add("c", 1)).and(addAll("d", new TreeSet<Integer>(){{ add(2); add(3); add(4); }}));
272+
update = update("foo").with(discardAll("b", Arrays.asList(1, 2, 3))).and(add("c", 1)).and(addAll("d", new TreeSet<Integer>() {{
273+
add(2);
274+
add(3);
275+
add(4);
276+
}}));
254277
assertEquals(update.toString(), query);
255278

256279
query = "UPDATE foo SET b=b-{2,3,4},c['k']='v',d=d+{'x':3,'y':2};";
257-
update = update("foo").with(removeAll("b", new TreeSet<Integer>(){{ add(2); add(3); add(4); }}))
258-
.and(put("c", "k", "v"))
259-
.and(putAll("d", new TreeMap<String, Integer>(){{ put("x", 3); put("y", 2); }}));
280+
update = update("foo").with(removeAll("b", new TreeSet<Integer>() {{
281+
add(2);
282+
add(3);
283+
add(4);
284+
}}))
285+
.and(put("c", "k", "v"))
286+
.and(putAll("d", new TreeMap<String, Integer>() {{
287+
put("x", 3);
288+
put("y", 2);
289+
}}));
260290
assertEquals(update.toString(), query);
261291

262292
query = "UPDATE foo USING TTL 400;";
263293
update = update("foo").using(ttl(400));
264294
assertEquals(update.toString(), query);
265295

266-
query = "UPDATE foo SET a="+new BigDecimal(3.2)+",b=42 WHERE k=2;";
296+
query = "UPDATE foo SET a=" + new BigDecimal(3.2) + ",b=42 WHERE k=2;";
267297
update = update("foo").with(set("a", new BigDecimal(3.2))).and(set("b", new BigInteger("42"))).where(eq("k", 2));
268298
assertEquals(update.toString(), query);
269299

@@ -280,7 +310,7 @@ public void updateTest() throws Exception {
280310
assertEquals(update.toString(), query);
281311

282312
try {
283-
update = update("foo").using(ttl(-400));
313+
update("foo").using(ttl(-400));
284314
fail();
285315
} catch (IllegalArgumentException e) {
286316
assertEquals(e.getMessage(), "Invalid ttl, must be positive");
@@ -326,14 +356,14 @@ public void deleteTest() throws Exception {
326356
assertEquals(delete.toString(), query);
327357

328358
try {
329-
delete = delete().column("a").all().from("foo");
359+
delete().column("a").all().from("foo");
330360
fail();
331361
} catch (IllegalStateException e) {
332362
assertEquals(e.getMessage(), "Some columns ([a]) have already been selected.");
333363
}
334364

335365
try {
336-
delete = delete().from("foo").using(timestamp(-1240003134L));
366+
delete().from("foo").using(timestamp(-1240003134L));
337367
fail();
338368
} catch (IllegalArgumentException e) {
339369
assertEquals(e.getMessage(), "Invalid timestamp, must be positive");
@@ -364,7 +394,11 @@ public void batchTest() throws Exception {
364394
query += "DELETE a[3],b['foo'],c FROM foo WHERE k=1;";
365395
query += "APPLY BATCH;";
366396
batch = batch()
367-
.add(insertInto("foo").values(new String[]{ "a", "b"}, new Object[]{ new TreeSet<Integer>(){{ add(2); add(3); add(4); }}, 3.4 }))
397+
.add(insertInto("foo").values(new String[]{ "a", "b" }, new Object[]{ new TreeSet<Integer>() {{
398+
add(2);
399+
add(3);
400+
add(4);
401+
}}, 3.4 }))
368402
.add(update("foo").with(setIdx("a", 2, "foo")).and(prependAll("b", Arrays.asList(3, 2, 1))).and(remove("c", "a")).where(eq("k", 2)))
369403
.add(delete().listElt("a", 3).mapElt("b", "foo").column("c").from("foo").where(eq("k", 1)))
370404
.using(timestamp(42));
@@ -451,12 +485,9 @@ public void batchCounterTest() throws Exception {
451485
assertEquals(batch.toString(), query);
452486
}
453487

454-
@Test(groups = "unit", expectedExceptions={IllegalArgumentException.class})
488+
@Test(groups = "unit", expectedExceptions = { IllegalArgumentException.class })
455489
public void batchMixedCounterTest() throws Exception {
456-
String query;
457-
Statement batch;
458-
459-
batch = batch()
490+
batch()
460491
.add(update("foo").with(incr("a", 1)))
461492
.add(update("foo").with(set("b", 2)))
462493
.add(update("foo").with(incr("c", 3)))
@@ -470,8 +501,8 @@ public void markerTest() throws Exception {
470501

471502
query = "INSERT INTO test(k,c) VALUES (0,?);";
472503
insert = insertInto("test")
473-
.value("k", 0)
474-
.value("c", bindMarker());
504+
.value("k", 0)
505+
.value("c", bindMarker());
475506
assertEquals(insert.toString(), query);
476507
}
477508

@@ -498,7 +529,6 @@ public void rawEscapingTest() throws Exception {
498529
assertEquals(select.toString(), query);
499530
}
500531

501-
502532
@Test(groups = "unit")
503533
public void selectInjectionTests() throws Exception {
504534

@@ -630,7 +660,7 @@ public void deleteInjectionTests() throws Exception {
630660

631661
query = "DELETE a,b FROM foo WHERE a IN ('b','c''); --comment');";
632662
delete = delete("a", "b").from("foo")
633-
.where(in("a", "b", "c'); --comment"));
663+
.where(in("a", "b", "c'); --comment"));
634664
assertEquals(delete.toString(), query);
635665

636666
query = "DELETE FROM foo WHERE \"k=1 OR k\">42;";
@@ -659,6 +689,7 @@ public void statementForwardingTest() throws Exception {
659689
public void rejectUnknownValueTest() throws Exception {
660690

661691
try {
692+
//noinspection ResultOfMethodCallIgnored
662693
update("foo").with(set("a", new byte[13])).where(eq("k", 2)).toString();
663694
fail("Byte arrays should not be valid, ByteBuffer should be used instead");
664695
} catch (IllegalArgumentException e) {
@@ -704,7 +735,7 @@ public void compoundWhereClauseTest() throws Exception {
704735

705736
query = "SELECT * FROM foo WHERE k=4 AND (c1,c2)>=('a',2) AND (c1,c2)<('b',0);";
706737
select = select().all().from("foo").where(eq("k", 4)).and(gte(Arrays.asList("c1", "c2"), Arrays.<Object>asList("a", 2)))
707-
.and(lt(Arrays.asList("c1", "c2"), Arrays.<Object>asList("b", 0)));
738+
.and(lt(Arrays.asList("c1", "c2"), Arrays.<Object>asList("b", 0)));
708739
assertEquals(select.toString(), query);
709740

710741
query = "SELECT * FROM foo WHERE k=4 AND (c1,c2)<=('a',2);";
@@ -771,6 +802,15 @@ public void should_handle_nested_collections() {
771802
assertThat(statement.toString()).isEqualTo(query);
772803
}
773804

805+
@Test(groups = "unit")
806+
public void should_quote_complex_column_names() {
807+
// A column name can be anything as long as it's quoted, so "foo.bar" is a valid name
808+
String query = "SELECT * FROM foo WHERE \"foo.bar\"=1;";
809+
Statement statement = select().from("foo").where(eq(quote("foo.bar"), 1));
810+
811+
assertThat(statement.toString()).isEqualTo(query);
812+
}
813+
774814
@Test(groups = "unit")
775815
public void should_not_serialize_raw_query_values() {
776816
RegularStatement select = select().from("test").where(gt("i", raw("1")));

0 commit comments

Comments
 (0)