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

RegExp.prototype.source getter called on non-RegExp object #3534

Closed
consense opened this issue Jan 28, 2019 · 5 comments · Fixed by #5182
Closed

RegExp.prototype.source getter called on non-RegExp object #3534

consense opened this issue Jan 28, 2019 · 5 comments · Fixed by #5182

Comments

@consense
Copy link

consense commented Jan 28, 2019

Issue type:

[ ] question
[ X ] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[ X ] postgres
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[ ] latest
[ ] @next
[ X ] typeorm@0.2.9 (or put your version here)

Error doesnt appear in typeorm@0.2.8 and is thrown in versions typeorm@0.2.9 through to typeorm@0.2.12

Steps to reproduce or a small repository showing the problem:

     TypeError: RegExp.prototype.source getter called on non-RegExp object
      at RegExp.get source [as source] (<anonymous>)
      at RegExp.toString (<anonymous>)
      at isNaN (<anonymous>)
      at Function.OrmUtils.compare2Objects (src\util\OrmUtils.ts:171:13)
      at Function.OrmUtils.compare2Objects (src\util\OrmUtils.ts:236:31)
      at Function.OrmUtils.compare2Objects (src\util\OrmUtils.ts:236:31)
      at Function.OrmUtils.compare2Objects (src\util\OrmUtils.ts:236:31)
      at Function.OrmUtils.deepCompare (src\util\OrmUtils.ts:118:23)
      at Function.EntityMetadata.compareIds (src\metadata\EntityMetadata.ts:723:25)
      at EntityMetadata.compareEntities (src\metadata\EntityMetadata.ts:590:31)
      at \src\persistence\SubjectDatabaseEntityLoader.ts:126:81
      at Array.filter (<anonymous>)
      at SubjectDatabaseEntityLoader.findByPersistEntityLike (src\persistence\SubjectDatabaseEntityLoader.ts:119:30)
      at \src\persistence\SubjectDatabaseEntityLoader.ts:96:39
      at Array.forEach (<anonymous>)
      at SubjectDatabaseEntityLoader.<anonymous> (src\persistence\SubjectDatabaseEntityLoader.ts:95:22)
      at step (node_modules\typeorm\persistence\SubjectDatabaseEntityLoader.js:32:23)
      at Object.next (node_modules\typeorm\persistence\SubjectDatabaseEntityLoader.js:13:53)
      at fulfilled (node_modules\typeorm\persistence\SubjectDatabaseEntityLoader.js:4:58)
      at process._tickCallback (internal/process/next_tick.js:68:7)

Gladly tell me if you'd need more info - tbh I would be hard pressed to produce a minimal reproduction repo, but if you have any specific questions just shoot :-)

@Kononnable
Copy link
Contributor

Yeah, I think we would need some way to reproduce that :)
Right now we know when bug was introduced(1 month period) but without way to reproduce it we won't be able to confirm any fixes or even if bug is in typeorm code.

@sam-hunt
Copy link

Just had this throw in typeorm@0.2.18. Also as yet unsure of the cause.

@tobyhinloopen
Copy link
Contributor

tobyhinloopen commented Dec 5, 2019

@Kononnable I found a way to reproduce the error: Try to persist an object with a RegExp in one of the columns. Note that I do use a transformer to transform a regular expression into a string.

import { Entity, PrimaryGeneratedColumn } from "typeorm";
import { RegExpColumn } from "../../src/RegExpColumn";

@Entity()
export class Thing {
  @PrimaryGeneratedColumn()
  public id: number;

  @RegExpColumn()
  public foo: RegExp;
}
import { Column, ColumnOptions } from "typeorm";
import { RegExpStringTransformer } from "./RegExpStringTransformer";

export function RegExpColumn(opts: ColumnOptions = {}) {
  opts.type = String;
  opts.transformer = RegExpStringTransformer;
  return Column(opts)
}
export namespace RegExpStringTransformer {
  export function to(value: RegExp): string {
    return value.toString(); // <<< error is thrown here, since `value` is no longer a RegExp
  }

  export function from(value: string): RegExp {
    const match = value.match(/^\/(.*)\/(.*)$/);
    if (match) {
      const [, pattern, flags] = match;
      return new RegExp(pattern, flags);
    } else {
      throw new Error(`"${value}" is not a regular expression`);
    }
  }
}
test("entity with RegExpColumn stores regular expression", async function() {
  const thing = new Thing();
  thing.foo = /foo/i;
  const savedThing = await thingRepo.save(thing); // <<< Error here
  expect(savedThing.foo).toEqual(/foo/i);
  const storedThing = await thingRepo.findOne(savedThing.id);
  expect(storedThing.foo).toEqual(/foo/i);
});

Repo (exact commit): https://github.com/tobyhinloopen/bonaroo-reg-exp-column/tree/65f591bffe62cbf9bdabd84cf4fc6352a5ae6eb5

I've done hours of traversing through typeorm and SOMEWHERE inside TypeORM something replaces the native /foo/i regexp with a seemingly empty object with a RegExp constructor. I suspect something tries to clone the entity or entity's values, and the cloning "breaks" the RegExp object:

$ node
> foo = /foo/i
/foo/i
> bar = Object.create(foo);
RegExp {}
> bar.toString()
TypeError: RegExp.prototype.source getter called on non-RegExp object
    at RegExp.get source [as source] (<anonymous>)
    at RegExp.toString (<anonymous>)

$ node -v
v10.15.1

@tobyhinloopen
Copy link
Contributor

tobyhinloopen commented Dec 5, 2019

Found the origin: OrmUtils.mergeDeep clones RegExp (and possibly other JS objects) incorrectly. This was last changed here:

c3ff3d3

tobyhinloopen added a commit to tobyhinloopen/typeorm that referenced this issue Dec 5, 2019
Before entity column values are transformed, changes are deeply merged using OrmUtils.mergeDeep. This mergeDeep function attempts to merge objects, but wrongly attempted to merge RegExp objects. This merging of RegExp objects breaks the object, rendering them unusable. This commit changes mergeDeep to not merge RegExp objects but overwrite them instead.

Closes typeorm#3534
@tobyhinloopen
Copy link
Contributor

tobyhinloopen commented Dec 5, 2019

@consense @sam-hunt This PR should fix your problem: #5182

tobyhinloopen added a commit to tobyhinloopen/typeorm that referenced this issue Dec 9, 2019
Before entity column values are transformed, changes are deeply merged using OrmUtils.mergeDeep. This mergeDeep function attempts to merge objects, but wrongly attempted to merge RegExp objects. This merging of RegExp objects breaks the object, rendering them unusable. This commit changes mergeDeep to not merge RegExp objects but overwrite them instead.

Closes typeorm#3534
pleerock added a commit that referenced this issue Jan 22, 2020
* fix: change OrmUtils.mergeDeep to not merge RegExp objects

Before entity column values are transformed, changes are deeply merged using OrmUtils.mergeDeep. This mergeDeep function attempts to merge objects, but wrongly attempted to merge RegExp objects. This merging of RegExp objects breaks the object, rendering them unusable. This commit changes mergeDeep to not merge RegExp objects but overwrite them instead.

Closes #3534

* fixing tests...

* fixing tslint

* Include import properly

* Fix TSLint

Co-authored-by: Umed Khudoiberdiev <pleerock.me@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants