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

Document jQuery.isEmptyObject doesn't take symbols into account #1192

Open
XiaoY0324 opened this issue Jun 6, 2021 · 2 comments
Open

Document jQuery.isEmptyObject doesn't take symbols into account #1192

XiaoY0324 opened this issue Jun 6, 2021 · 2 comments

Comments

@XiaoY0324
Copy link

Description

On version 3.6.0, I found a vulnerability,I find that the method of detecting empty objects is not perfect.

isEmptyObject: function( obj ) {
  var name;

  for ( name in obj ) {
     return false;
   }
   return true;
},

The downside of writing this is that the properties of the prototype chain and the Symbol type are not taken into account,I have a better way to implement it as follows.

isEmptyObject: function( obj ) {
    var keys = Object.keys(obj);

    // 看浏览器是否支持 Symbol
    if (typeof Symbol !== "undefined") {
       keys = keys.concat(Object.getOwnPropertySymbols(obj));
    }
  
    return keys.length === 0;
}

Link to test case

This direct modification of the source code test is good, I do not have a service to run this test. I'm looking forward to hearing from you.

@mgol
Copy link
Member

mgol commented Jun 6, 2021

The docs for jQuery.isEmptyObject mention what is considered an empty object (emphasis mine):

Description: Check to see if an object is empty (contains no enumerable properties).

The description doesn't account for symbols as it was written long ago. That said, changing the existing behavior would be a breaking change and I think it's better to just update the docs here. I'll mark it for a team discussion.

BTW, what do you mean that you found a vulnerability? You explained a difference between your expectations and the library behavior, I don't see any security issue here.

@timmywil
Copy link
Member

timmywil commented Jun 7, 2021

We'll make this an API issue and update the wording to mention Symbols and possibly "string keyed properties" like lodash.

@mgol mgol changed the title A better way to implement it with isEmptyObject function Document jQuery.isEmptyObject doesn't take symbols into account Jun 7, 2021
@mgol mgol transferred this issue from jquery/jquery Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants