diff --git a/src/query-builder/QueryBuilder.ts b/src/query-builder/QueryBuilder.ts index 004df423ef..f11ac46a98 100644 --- a/src/query-builder/QueryBuilder.ts +++ b/src/query-builder/QueryBuilder.ts @@ -20,6 +20,7 @@ import {OracleDriver} from "../driver/oracle/OracleDriver"; import {EntitySchema} from "../"; import {FindOperator} from "../find-options/FindOperator"; import {In} from "../find-options/operator/In"; +import {EntityColumnNotFound} from "../error/EntityColumnNotFound"; // todo: completely cover query builder with tests // todo: entityOrProperty can be target name. implement proper behaviour if it is. @@ -788,6 +789,11 @@ export abstract class QueryBuilder { return propertyPaths.map((propertyPath, propertyIndex) => { const columns = this.expressionMap.mainAlias!.metadata.findColumnsWithPropertyPath(propertyPath); + + if (!columns.length) { + throw new EntityColumnNotFound(propertyPath); + } + return columns.map((column, columnIndex) => { const aliasPath = this.expressionMap.aliasNamePrefixingEnabled ? `${this.alias}.${propertyPath}` : column.propertyPath; diff --git a/test/functional/query-builder/delete/query-builder-delete.ts b/test/functional/query-builder/delete/query-builder-delete.ts index 6bc071130c..05d091dc46 100644 --- a/test/functional/query-builder/delete/query-builder-delete.ts +++ b/test/functional/query-builder/delete/query-builder-delete.ts @@ -4,6 +4,7 @@ import {closeTestingConnections, createTestingConnections, reloadTestingDatabase import {Connection} from "../../../../src/connection/Connection"; import {User} from "./entity/User"; import {Photo} from "./entity/Photo"; +import {EntityColumnNotFound} from "../../../../src/error/EntityColumnNotFound"; describe("query builder > delete", () => { @@ -113,4 +114,24 @@ describe("query builder > delete", () => { expect(result.affected).to.equal(2); }))); + it("should throw error when unknown property in where criteria", () => Promise.all(connections.map(async connection => { + + const user = new User(); + user.name = "Alex Messer"; + + await connection.manager.save(user); + + let error: Error | undefined; + try { + await connection.createQueryBuilder() + .delete() + .from(User) + .where( { unknownProp: "Alex Messer" }) + .execute(); + } catch (err) { + error = err; + } + expect(error).to.be.an.instanceof(EntityColumnNotFound); + + }))); }); diff --git a/test/functional/query-builder/update/query-builder-update.ts b/test/functional/query-builder/update/query-builder-update.ts index 17c55ee509..da4c33dabc 100644 --- a/test/functional/query-builder/update/query-builder-update.ts +++ b/test/functional/query-builder/update/query-builder-update.ts @@ -249,4 +249,25 @@ describe("query builder > update", () => { }))); + it("should throw error when unknown property in where criteria", () => Promise.all(connections.map(async connection => { + + const user = new User(); + user.name = "Alex Messer"; + + await connection.manager.save(user); + + let error: Error | undefined; + try { + await connection.createQueryBuilder() + .update(User) + .set({ name: "John Doe" } as any) + .where( { unknownProp: "Alex Messer" }) + .execute(); + } catch (err) { + error = err; + } + expect(error).to.be.an.instanceof(EntityColumnNotFound); + + }))); + }); diff --git a/test/github-issues/3416/entity/User.ts b/test/github-issues/3416/entity/User.ts new file mode 100644 index 0000000000..c8e2ff0060 --- /dev/null +++ b/test/github-issues/3416/entity/User.ts @@ -0,0 +1,17 @@ +import {Entity} from "../../../../src/decorator/entity/Entity"; +import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn"; +import {Column} from "../../../../src/decorator/columns/Column"; + +@Entity() +export class User { + + @PrimaryGeneratedColumn() + id: number; + + @Column() + name: string; + + @Column() + likesCount: number = 0; + +} diff --git a/test/github-issues/3416/issue-3416.ts b/test/github-issues/3416/issue-3416.ts new file mode 100644 index 0000000000..671be02934 --- /dev/null +++ b/test/github-issues/3416/issue-3416.ts @@ -0,0 +1,47 @@ +import "reflect-metadata"; +import { expect } from "chai"; +import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils"; +import { Connection } from "../../../src/connection/Connection"; +import {User} from "../../functional/query-builder/update/entity/User"; +import {EntityColumnNotFound} from "../../../src/error/EntityColumnNotFound"; + +describe("github issues > #3416 Unknown fields are stripped from WHERE clause", () => { + + let connections: Connection[]; + before(async () => connections = await createTestingConnections({ + entities: [User] + })); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + describe("should throw EntityColumnNotFound when supplying unknown property in where criteria", () => { + it("find", () => Promise.all(connections.map(async connection => { + let error: Error | undefined; + try { + // @ts-ignore + await connection.manager.findOne(User, {unknownProp: "John Doe"}); + } catch (err) { + error = err; + } + expect(error).to.be.an.instanceof(EntityColumnNotFound); + }))); + it("update", () => Promise.all(connections.map(async connection => { + let error: Error | undefined; + try { + await connection.manager.update(User, { unknownProp: "Something" }, { name: "John doe "}); + } catch (err) { + error = err; + } + expect(error).to.be.an.instanceof(EntityColumnNotFound); + }))); + it("delete", () => Promise.all(connections.map(async connection => { + let error: Error | undefined; + try { + await connection.manager.delete(User, { unknownProp: "Something" }); + } catch (err) { + error = err; + } + expect(error).to.be.an.instanceof(EntityColumnNotFound); + }))); + }); +}); diff --git a/test/github-issues/3654/entity/User.ts b/test/github-issues/3654/entity/User.ts deleted file mode 100644 index 99a0a9ee7f..0000000000 --- a/test/github-issues/3654/entity/User.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { StringDecoder } from "string_decoder"; -import { Column, Entity, PrimaryColumn } from "../../../../src"; - -@Entity() -export class User { - - @PrimaryColumn("binary", { - length: 16 - }) - public _id: Buffer; - get id(): string { - const decoder = new StringDecoder("hex"); - - return decoder.end(this._id); - } - set id(value: string) { - this._id = Buffer.from(value, "hex"); - } - - @Column() - age: number; - -} diff --git a/test/github-issues/3654/issue-3654.ts b/test/github-issues/3654/issue-3654.ts deleted file mode 100644 index b1f309eb10..0000000000 --- a/test/github-issues/3654/issue-3654.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { Connection } from "../../../src"; -import { - closeTestingConnections, - createTestingConnections, - reloadTestingDatabases -} from "../../utils/test-utils"; -import { User } from "./entity/User"; - -describe("github issues > #3654 Should be able compare buffer type", () => { - let connections: Connection[]; - before( - async () => - (connections = await createTestingConnections({ - entities: [__dirname + "/entity/*{.js,.ts}"], - enabledDrivers: ["mysql"] - })) - ); - beforeEach(() => reloadTestingDatabases(connections)); - after(() => closeTestingConnections(connections)); - - it("Repository.save() method should be able compare buffer type for deciding if save or update ops.", () => - Promise.all( - connections.map(async connection => { - const userRepo = connection.getRepository(User); - - let userId = "4321226123455910A532153E57A78445".toLowerCase(); - - const user = new User(); - user.id = userId; - user.age = 25; - await userRepo.save(user); - - const dbUser = (await userRepo.find({ - where: { - id: Buffer.from(userId, "hex") - } - }))[0]; - - dbUser.age = 26; - await userRepo.save(dbUser); - - const confirmUser = (await userRepo.find({ - where: { - id: Buffer.from(userId, "hex") - } - }))[0]; - - confirmUser.id.should.be.eql(userId); - confirmUser.age.should.be.eql(26); - }) - )); -}); diff --git a/test/other-issues/preventing-injection/preventing-injection.ts b/test/other-issues/preventing-injection/preventing-injection.ts index c98afeec69..028dfaebe7 100644 --- a/test/other-issues/preventing-injection/preventing-injection.ts +++ b/test/other-issues/preventing-injection/preventing-injection.ts @@ -3,6 +3,7 @@ import {closeTestingConnections, createTestingConnections, reloadTestingDatabase import {Connection} from "../../../src"; import {Post} from "./entity/Post"; import {expect} from "chai"; +import {EntityColumnNotFound} from "../../../src/error/EntityColumnNotFound"; describe("other issues > preventing-injection", () => { @@ -28,7 +29,7 @@ describe("other issues > preventing-injection", () => { }).should.be.rejected; }))); - it("should skip non-exist columns in where expression via FindOptions", () => Promise.all(connections.map(async function(connection) { + it("should throw error for non-exist columns in where expression via FindOptions", () => Promise.all(connections.map(async function(connection) { const post = new Post(); post.title = "hello"; await connection.manager.save(post); @@ -40,13 +41,18 @@ describe("other issues > preventing-injection", () => { }); postWithOnlyIdSelected.should.be.eql([{ id: 1, title: "hello" }]); - const loadedPosts = await connection.manager.find(Post, { - where: { - id: 2, - ["(WHERE LIMIT 1)"]: "hello" - } - }); - loadedPosts.should.be.eql([]); + let error: Error | undefined; + try { + await connection.manager.find(Post, { + where: { + id: 2, + ["(WHERE LIMIT 1)"]: "hello" + } + }); + } catch (err) { + error = err; + } + expect(error).to.be.an.instanceof(EntityColumnNotFound); }))); it("should not allow selection of non-exist columns via FindOptions", () => Promise.all(connections.map(async function(connection) {