Skip to content

Commit 215f106

Browse files
johannessjobergjohannessjoberg
authored
fix: Unknown fields are stripped from WHERE clause (issue #3416) (#5603)
* fix: Unknown fields are stripped from WHERE clause (issue #3416) * update non-exist columns test
1 parent 3dc9ffd commit 215f106

File tree

8 files changed

+126
-83
lines changed

8 files changed

+126
-83
lines changed

‎src/query-builder/QueryBuilder.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {OracleDriver} from "../driver/oracle/OracleDriver";
2020
import {EntitySchema} from "../";
2121
import {FindOperator} from "../find-options/FindOperator";
2222
import {In} from "../find-options/operator/In";
23+
import {EntityColumnNotFound} from "../error/EntityColumnNotFound";
2324

2425
// todo: completely cover query builder with tests
2526
// todo: entityOrProperty can be target name. implement proper behaviour if it is.
@@ -788,6 +789,11 @@ export abstract class QueryBuilder<Entity> {
788789

789790
return propertyPaths.map((propertyPath, propertyIndex) => {
790791
const columns = this.expressionMap.mainAlias!.metadata.findColumnsWithPropertyPath(propertyPath);
792+
793+
if (!columns.length) {
794+
throw new EntityColumnNotFound(propertyPath);
795+
}
796+
791797
return columns.map((column, columnIndex) => {
792798

793799
const aliasPath = this.expressionMap.aliasNamePrefixingEnabled ? `${this.alias}.${propertyPath}` : column.propertyPath;

‎test/functional/query-builder/delete/query-builder-delete.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {closeTestingConnections, createTestingConnections, reloadTestingDatabase
44
import {Connection} from "../../../../src/connection/Connection";
55
import {User} from "./entity/User";
66
import {Photo} from "./entity/Photo";
7+
import {EntityColumnNotFound} from "../../../../src/error/EntityColumnNotFound";
78

89
describe("query builder > delete", () => {
910

@@ -113,4 +114,24 @@ describe("query builder > delete", () => {
113114
expect(result.affected).to.equal(2);
114115
})));
115116

117+
it("should throw error when unknown property in where criteria", () => Promise.all(connections.map(async connection => {
118+
119+
const user = new User();
120+
user.name = "Alex Messer";
121+
122+
await connection.manager.save(user);
123+
124+
let error: Error | undefined;
125+
try {
126+
await connection.createQueryBuilder()
127+
.delete()
128+
.from(User)
129+
.where( { unknownProp: "Alex Messer" })
130+
.execute();
131+
} catch (err) {
132+
error = err;
133+
}
134+
expect(error).to.be.an.instanceof(EntityColumnNotFound);
135+
136+
})));
116137
});

‎test/functional/query-builder/update/query-builder-update.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,4 +249,25 @@ describe("query builder > update", () => {
249249

250250
})));
251251

252+
it("should throw error when unknown property in where criteria", () => Promise.all(connections.map(async connection => {
253+
254+
const user = new User();
255+
user.name = "Alex Messer";
256+
257+
await connection.manager.save(user);
258+
259+
let error: Error | undefined;
260+
try {
261+
await connection.createQueryBuilder()
262+
.update(User)
263+
.set({ name: "John Doe" } as any)
264+
.where( { unknownProp: "Alex Messer" })
265+
.execute();
266+
} catch (err) {
267+
error = err;
268+
}
269+
expect(error).to.be.an.instanceof(EntityColumnNotFound);
270+
271+
})));
272+
252273
});

‎test/github-issues/3416/entity/User.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {Entity} from "../../../../src/decorator/entity/Entity";
2+
import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
3+
import {Column} from "../../../../src/decorator/columns/Column";
4+
5+
@Entity()
6+
export class User {
7+
8+
@PrimaryGeneratedColumn()
9+
id: number;
10+
11+
@Column()
12+
name: string;
13+
14+
@Column()
15+
likesCount: number = 0;
16+
17+
}

‎test/github-issues/3416/issue-3416.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import "reflect-metadata";
2+
import { expect } from "chai";
3+
import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils";
4+
import { Connection } from "../../../src/connection/Connection";
5+
import {User} from "../../functional/query-builder/update/entity/User";
6+
import {EntityColumnNotFound} from "../../../src/error/EntityColumnNotFound";
7+
8+
describe("github issues > #3416 Unknown fields are stripped from WHERE clause", () => {
9+
10+
let connections: Connection[];
11+
before(async () => connections = await createTestingConnections({
12+
entities: [User]
13+
}));
14+
beforeEach(() => reloadTestingDatabases(connections));
15+
after(() => closeTestingConnections(connections));
16+
17+
describe("should throw EntityColumnNotFound when supplying unknown property in where criteria", () => {
18+
it("find", () => Promise.all(connections.map(async connection => {
19+
let error: Error | undefined;
20+
try {
21+
// @ts-ignore
22+
await connection.manager.findOne(User, {unknownProp: "John Doe"});
23+
} catch (err) {
24+
error = err;
25+
}
26+
expect(error).to.be.an.instanceof(EntityColumnNotFound);
27+
})));
28+
it("update", () => Promise.all(connections.map(async connection => {
29+
let error: Error | undefined;
30+
try {
31+
await connection.manager.update(User, { unknownProp: "Something" }, { name: "John doe "});
32+
} catch (err) {
33+
error = err;
34+
}
35+
expect(error).to.be.an.instanceof(EntityColumnNotFound);
36+
})));
37+
it("delete", () => Promise.all(connections.map(async connection => {
38+
let error: Error | undefined;
39+
try {
40+
await connection.manager.delete(User, { unknownProp: "Something" });
41+
} catch (err) {
42+
error = err;
43+
}
44+
expect(error).to.be.an.instanceof(EntityColumnNotFound);
45+
})));
46+
});
47+
});

‎test/github-issues/3654/entity/User.ts‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎test/github-issues/3654/issue-3654.ts‎

Lines changed: 0 additions & 52 deletions
This file was deleted.

‎test/other-issues/preventing-injection/preventing-injection.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {closeTestingConnections, createTestingConnections, reloadTestingDatabase
33
import {Connection} from "../../../src";
44
import {Post} from "./entity/Post";
55
import {expect} from "chai";
6+
import {EntityColumnNotFound} from "../../../src/error/EntityColumnNotFound";
67

78
describe("other issues > preventing-injection", () => {
89

@@ -28,7 +29,7 @@ describe("other issues > preventing-injection", () => {
2829
}).should.be.rejected;
2930
})));
3031

31-
it("should skip non-exist columns in where expression via FindOptions", () => Promise.all(connections.map(async function(connection) {
32+
it("should throw error for non-exist columns in where expression via FindOptions", () => Promise.all(connections.map(async function(connection) {
3233
const post = new Post();
3334
post.title = "hello";
3435
await connection.manager.save(post);
@@ -40,13 +41,18 @@ describe("other issues > preventing-injection", () => {
4041
});
4142
postWithOnlyIdSelected.should.be.eql([{ id: 1, title: "hello" }]);
4243

43-
const loadedPosts = await connection.manager.find(Post, {
44-
where: {
45-
id: 2,
46-
["(WHERE LIMIT 1)"]: "hello"
47-
}
48-
});
49-
loadedPosts.should.be.eql([]);
44+
let error: Error | undefined;
45+
try {
46+
await connection.manager.find(Post, {
47+
where: {
48+
id: 2,
49+
["(WHERE LIMIT 1)"]: "hello"
50+
}
51+
});
52+
} catch (err) {
53+
error = err;
54+
}
55+
expect(error).to.be.an.instanceof(EntityColumnNotFound);
5056
})));
5157

5258
it("should not allow selection of non-exist columns via FindOptions", () => Promise.all(connections.map(async function(connection) {

0 commit comments

Comments
 (0)