Routing-controllers: class transformer not stripping additional properties

[adnan-kamili]

As per the doc, a new instance of class is generated by the class-transformer, then why it contains additional properties. For the following post data:

{
    "email": "[email protected]",
    "name": "test name",
    "password": "password4",
    "company": "My Company"
}

And following ViewModel:

export class UserViewModel {

    @MaxLength(256)
    name: string;

    @IsEmail()
    email: string;

    @MinLength(6)
    @MaxLength(72)  // bcrypt input limit
    password: string;

    roles: Array<string>
}

The class instance generated for

    @HttpCode(201)
    @Post()
    create( @Body() user: UserViewModel) {
        console.log(user);
        return { message: "created"} ;
    }

is

UserViewModel {
  email: '[email protected]',
  name: 'test name',
  password: 'password4',
  company: 'My Company' }

Where the expected object was:

UserViewModel {
  email: '[email protected]',
  name: 'test name',
  password: 'password4',
  roles: null }

[NoNameProvided]

Only known properties are checked, others will be left untouched. If you want to exclude one specifically you can use the @Exclude() decorator.

[adnan-kamili]

My concern is how is it an instance of UserViewModel if it doesn't fulfill the contract. User can add in fields like createtAt, updatedAt etc.in the post body which can cause undesired effects.

What we expect is a class instance not what the user provided json parsed object, to ensure safe data is saved into the database. Adding @Exclude() decorator for properties which I don't expect is not intuitive, and unnecessarily would require adding all the properties to the view model which I don't want from users/attackers.

[NoNameProvided]

What we expect is a class instance

It is a class instance, all your functions specified in your class will be there.

to ensure safe data is saved into the database

Hmm, so adding @Exclude() won't work when saving the object into the database. It's used to strip properties when serializing them before sending down to client. (via classToPlain function)

Adding @Exclude() decorator for properties which I don't expect is not intuitive

That is both true and false, while it's never bad to explicitly says which properties are forbidden, it is a manual work to add them. This manual work can be easily lowered by extending your class likeexport class UpdateUserPayload extends ForbiddenFields. However again, this wont save you from saving the wrong data in the database.

The solution would be to add a @StictEquals or similar class decorator to allow only the selected fields and throw otherwise, and a @Equals to simply strip down the unknow properties.

@pleerock what do you think?

[adnan-kamili]

All validation libraries like Joi, ajv, validate.js support stripping of properties which are not part of the schema, so this should be supported globally as a flag for class-transformer instead of adding one more decorator to each action. Adding too many decorators in itself also has an overhead for each action when number of actions is very high.

[pleerock]

I didn't quite understood why you are talking about exclude here?

What I understood from this message:

UserViewModel {
  email: '[email protected]',
  name: 'test name',
  password: 'password4',
  company: 'My Company' }
Where the expected object was:

UserViewModel {
  email: '[email protected]',
  name: 'test name',
  password: 'password4',
  roles: null }

Is that he expects roles to be null, but class-transformer makes it undefined. Do you I get it right? If yes, then its expected behaviour and null and undefined are different things and since you don't send in body roles at all you should not have it in transformed object. Or did I understand it wrongly?

[NoNameProvided]

Is that he expects roles to be null, but class-transformer makes it undefined. Do you I get it right?

No, the raised issue is about not removing the company property. It's doesn't exists on the model yet it exits on the transformed object. This is problematic in places like Mongo, when you will end up storing data you didn't want to.

[pleerock]

Thats why people must use typeorm to store documents where you mark each column you want to store a with a column decorator 😃

Okay but seriously its not possible to implement this feature.

In TypeScript its NOT POSSIBLE TO GET KNOWN WHAT PROPERTIES ARE DEFINED IN THE CLASS.
It means we simply don't know if your model has or not "company" property.

[MichalLytek]

Why not? If the property is decorated with class-validator decorator (even @IsOptional()) we can just do:

const reflectedType = Reflect.getMetadata("design:type", UserViewModel.prototype, "company");
if (!reflectedType) {
    delete object["company"];
} else {
    if (object["company"].constructor !== reflectedType) {
        throw new PropertyConveringError();
    }
}

I would like to introduce this type of strictTransform function to class-transformer, which would require decorating each property to collect type but the gain is that we can validate the type of the property and strip additional ones + throw error when some of object properties are invalid (string expected, get boolean instead).

BTW, to get all decorated class property names we can even monkey-patch Reflect.defineMetadata to collect metadata into storage 😆

[pleerock]

If you read comment above author would like to avoid decorating all properties. There is already support in class-transformer to serialize only decoratorated properties (using @Expose decorator).

[MichalLytek]

Serialize yes, but deserialize? The problem is that the received body has additional properties and all that class-transformer do is assign object prototype, not check or strip additional properties, so when you want to store it in db you have to serialize and deserialize again? It makes no sense, it should be done by strictTransform function.

[pleerock]

it works for both serialize and deserialize.

[MichalLytek]

Oh rly?

For me, strictTransform should strip the additional property and leave only the ones declared (and decorated) in class + throw error when the type won't match (2 !== "2").

Having additional properties is an issue for storing object in documents but the horrible thing is that the types are not checked so you have completely no type safe after transforming, it's just methods attaching which isn't very helpful feature in DTO.

[pleerock]

As I remember @Expose does not work until u mark class with @Exclude() decorator:

@Exclude()
export class Test {

  @Expose()
   property: number

}

Or alternatively

export class Test {

  @Expose()
   property: number

}

and

plainToClass(Test, { ... }, { strategy: "excludeAll" });

[MichalLytek]

Ok, it works even without excludeAll strategy:

But the problem with no type-check is still there - if you don't mind, I will do the PR with strict option which would check types and throw error if doesn't match, as it's a breaking change.

[MichalLytek]

In TypeScript its NOT POSSIBLE TO GET KNOWN WHAT PROPERTIES ARE DEFINED IN THE CLASS.

Here is my patch:

const _metadata = Reflect.metadata
Reflect.metadata = function(metadataKey: any, metadataValue: any) {
    if (metadataKey === "design:type") {
        return function decorator(target: any, propertyKey?: string | symbol): void {
            const designTypes = Reflect.getMetadata("design:types", target) || []
            designTypes.push({
                propertyKey,
                type: metadataValue,
            })
            Reflect.defineMetadata("design:types", designTypes, target);
            _metadata.call(Reflect, metadataKey, metadataValue)(target, propertyKey)
        }
    }
    return _metadata.apply(Reflect, arguments);
}

I will release it as reflect-metadata-types npm package, so the only thing to do is to register on top of entry file:

import "reflect-metatada";
import "reflect-metatada-types";

And then you can just simple do:

Reflect.getOwnMetadata("design:types", Test.prototype)

Which will return you an array of properties/methods with types. Example:

@ClassDec()
export class Test {
    @PropDec()
    one: string;
    @PropDec()
    two: number;
    @PropDec()
    three: boolean;
    @PropDec()
    method() {
        return "Hello"
    }
}

[
  { propertyKey: 'one', type: [λ: String] },​​​​​
​​​​​  { propertyKey: 'two', type: [λ: Number] },​​​​​
​​​​​  { propertyKey: 'three', type: [λ: Boolean] },​​​​​
​​​​​  { propertyKey: 'method', type: [λ: Function] }
]​​​​​

[pleerock]

yeah we can get all properties if they marked with decorator even without any other dependency, but again its not an option. If someone needs such option it already exist in class-transformer

[MichalLytek]

yeah we can get all properties if they marked with decorator even without any other dependency, but again its not an option

Not really. Can you get all properties names and types in class-transformer when the class is decorated only with typeorm/class-validator decorators? The types metadatas are emmited for each property but you can't access it by class type value. My workaround allows you to collect all properties with types if emmited metadata, not only the ones decorated with @Type or other class-transformer decorators when using it is not neccesary.

If someone needs such option it already exist in class-transformer

But this could be done without bloating the class markup. Currently it has to look like this:

@Exclude()
export class CustomClass {
    @Length(3, 255)
    @Expose()
    one: string;

    @IsInt()
    @Expose()
    two: number;

    @IsOptional()
    @Expose()
    isNice?: boolean = true;
}

But could look better:

class CustomClass {
    @Length(3, 255)
    one: string;

    @IsInt()
    two: number;

    @IsOptional()
    isNice?: boolean = true;
}

[pleerock]

Same ideas and thoughts I had a whole time ago. Its not a solution.
Most of times class properties does not have any decorators and you will end up having:

class CustomClass {
    @Length(3, 255)
    one: string;

    @IsInt()
    two: number;

    @IsOptional()
    isNice?: boolean = true;

    @PropDec()
    a: string;

    @PropDec()
    b: string;
}

which ends up as a mess and non clear and very very implicit about what this @PropDec() does and why others dont have it.

[MichalLytek]

which ends up as a mess and non clear and very very implicit about what this @propdec() does and why others dont have it.

Nope, for empty properties with no constraints you can use @IsDefined or @IsOptional decorator when class is used for validation purpose.

class City {
    @Length(3, 255)
    name: string;

    @IsOptional()
    isCaptial?: boolean = true;

    @IsDefined()
    density: number;
}

Other decorators like @Length or @IsInt implicitly have constraint that the prop @IsDefined().

Also, the empty @PropDec() could have better name, like @CollectType which would be described nicely how TS reflection works and when to use it.

[pleerock]

Mess in anyway. You don't need validation in most models, no matter how propdec is called it will look dirty anyway, patching with is defined or is optional no matter decorators when you dont want to do such validation is dirty too. Mess and dirty.

[marshall007]

@19majkel94 @pleerock FYI, per https://github.com/Microsoft/TypeScript/issues/12437 it looks like the latest ES proposal states that class properties will be initialized with undefined values by default. In the future, we may be able to rely on that for looping over non-decorated class properties.

[NoNameProvided]

In TypeScript its NOT POSSIBLE TO GET KNOWN WHAT PROPERTIES ARE DEFINED IN THE CLASS.

@pleerock Payloads are stateless so why we cant check it at the first conversion the properties on a dummy class?

// pseudo code below
function convert(type, obj) {
  if(!propertyMap[type]) {
    const instance new Type();
    propertyMap[type] = Object.getOwnPropertyNames(obj);
  }

  doStrictTransform(type, obj, propertyMap[type]);
}

[pleerock]

@NoNameProvided what do you mean? Im talking to get properties of class, not objects.

[NoNameProvided]

Im talking to get properties of class, not objects.

Classes are objects as well.

class TestClass {
  constructor() {
    this.x = undefined
  } 
}

Object.getOwnPropertyNames(TestClass);
// returns (3) ["length", "prototype", "name"]

But anyway I missed @marshall007 post about compilation of un-inited properties. So my approach wont work as

class TestClass {
  public x: string;
}

is compiled to

class TestClass {
  constructor() {  } 
}

and not

class TestClass {
  constructor() {
    this.x = undefined
  } 
}

[pleerock]

right

[styxiak]

@pleerock your solution from comment breaks first test form basic-functionality.spec.ts.
When we heave User object:

      @Exclude()
      class User {
        @Expose()
        id: number;
        @Expose()
        firstName: string;
        @Expose()
        lastName: string;
        @Exclude()
        password: string;
      }
const fromExistUser = new User();
        fromExistUser.id = 1;

and plain object:

const fromPlainUser = {
            firstName: "Umed",
            lastName: "Khudoiberdiev",
            password: "imnosuperman",
            additional: "something we dont want"
        };

then

const fromExistTransformedUser = plainToClassFromExist(fromExistUser, fromPlainUser);

will produce

fromExistTransformedUser = {
  id = undefined,
  firstName: "Umed",
  lastName: "Khudoiberdiev"
}

I don't know is it expected behaviour, but if, then will be nice to have option to omit undefined properties.

[NoNameProvided]

The latest release contains [email protected] which supports stripping unknown properties.

[hgrewa]

stripping unknown properties still do not address following
class A { a = 'a'; }
class B extends A { b = 'b'; }

object to be parsed
obj = const obj = { a: 'a', b: 'b', c: 'c' };

the desired result after the transform B is, { a: 'a', b: 'b }

plainToClass(obj, { strategy: "exposeAll" } will give { a: 'a', b: 'b', c: 'c' };
plainToClass(obj, { strategy: "excludeAll" } will give { b: 'b' };

[IRCraziestTaxi]

Just bumping this because I was also just searching for the solution to this. For instance, I am trying to use mediator pattern in my API, and if a user can just add a property in Postman, etc. that exists on the entity but not on the Command class, then without manual mapping of properties in every handler in order to ensure unwanted properties from the request are not persisted to the database, there is no way to ensure the user cannot do so.

Edit: I discovered that using the @Expose decorator on properties in my Command classes along with @Body({ transform: { strategy: "excludeAll" } }) to decorate the injected request body in my controller method will only include the properties in that class. I didn't realize at first that the comment by @hgrewa above me pertained to inheritance issues. In my case, the decorator and option I mentioned work for me.

Edit 2: Just found in the docs for class-transformer that the options passed into @Body can be replaced by an @Exclude decorator on the Command class, so that only the class properties (marked with @Expose are parsed from the request body. Much cleaner.

[vellengs]

Any news about this?

[ljobse]

For people who are using this in combination with class-validator and thus are decorating the objects with @IsString() for example anyway.

I wrote a little extension that uses those decorators to strip all unknown properties from the object.
https://gist.github.com/ljobse/a617ca62e7d0277a8da351479656f04f

The file just needs to be loaded and the plainToClass function will now strip all properties which are not decorated with anything.

[github-actions[bot]]

Stale issue message

[LukaszKuciel]

TBH, I don't get the point why plainToClass is not stripping objects off fields that are not defined in a given class. At least there should be an option to omit unknown values.
A very popular case for class-transformer is serialization. Let's say that you have many DTOs and if I understand correctly the only way now to have a properly serialized response according to DTO is to add @Expose() decorator to each field? This approach has no added value to it and it only makes developer life harder.
Please provide the option to skip fields that are not defined in the class definition.