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.

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

fix: Unknown fields are stripped from WHERE clause (issue #3416) #5603

Merged
merged 2 commits into from May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/query-builder/QueryBuilder.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -788,6 +789,11 @@ export abstract class QueryBuilder<Entity> {

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;
Expand Down
21 changes: 21 additions & 0 deletions test/functional/query-builder/delete/query-builder-delete.ts
Expand Up @@ -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", () => {

Expand Down Expand Up @@ -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);

})));
});
21 changes: 21 additions & 0 deletions test/functional/query-builder/update/query-builder-update.ts
Expand Up @@ -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);

})));

});
17 changes: 17 additions & 0 deletions 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;

}
47 changes: 47 additions & 0 deletions 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);
})));
});
});
23 changes: 0 additions & 23 deletions test/github-issues/3654/entity/User.ts

This file was deleted.

52 changes: 0 additions & 52 deletions test/github-issues/3654/issue-3654.ts

This file was deleted.

22 changes: 14 additions & 8 deletions test/other-issues/preventing-injection/preventing-injection.ts
Expand Up @@ -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", () => {

Expand All @@ -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);
Expand All @@ -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) {
Expand Down