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

class member method named "import" will be misrecognized as import statement. #773

Closed
softmarshmallow opened this issue Jun 11, 2021 · 5 comments

Comments

@softmarshmallow
Copy link

softmarshmallow commented Jun 11, 2021

figma/plugin-typings#36

  1. Read the above thread.

case 1.

/// Throws
Class A{
   import(a: string){console.log(a)}
}

/// Works
Class A{
   imports(a: string){console.log(a)} // or any other method name rather than "import"
}

case 2.


// Throws
// import something blahblah

// Works
// _import something blahblah
// `import` something blahblah

Improvement on regex and other logic barrier required.

@erights
Copy link
Contributor

erights commented Jun 12, 2021

See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_IMPORT_REJECTED.md

If this is code you have control over, for the code above, quoting import is your best bet.

@softmarshmallow
Copy link
Author

The endojs is for secure platforms (as far as i understand), which it's design should not affect anonymous package's specific syntax.

  1. blocking dynamic import - agree - since dynamic import is mostly used on end user level (end developer)
  2. blocking member function named import - disagree - since class member can be named anything as it can be and not allowing this violates the concept of class.
  3. import keyword in comment - needs improvement

@kriskowal
Copy link
Member

kriskowal commented Jun 15, 2021

@softmarshmallow For performance and security reasons, SES cannot include and rely upon a full JavaScript parser to distinguish all of the valid ways import( can occur within client code. Instead, it uses a regular expression with false-positives. Some of the false positives you’ve found are ones we haven’t seen yet! But, the mitigation in all of these cases is to do some preprocessing on the code you’re presenting to SES so that it can distinguish valid use.

For example @agoric/bundle-source has a parser-based transform that replaces import() expressions in comments with importX(). We had a problem with object.import() method invocation that we solved by making the censor regular expression more clever, but previously had transformed all such cases to object['import']() to avoid censorship. The pre-processor ought to add a case for your import “concise method”, changing those to import: () => {} or ['import']() {} concise methods.

We cannot address this problem with SES, because of that limitation on parser-based censorship, but we could write a “parser-based censorship evasion transform” over in the bundler. To benefit from this, your code would have to pass through the bundler transforms. I’ve filed this issue on your behalf:

Agoric/agoric-sdk#3325

@softmarshmallow
Copy link
Author

softmarshmallow commented Jun 15, 2021

IMHO, But still, I can't get that the parser scans the "import" keyword in the comment string. this is simply wrong.
Why would the regex target the safe, commented, code?

Same question, same statement. I can't understand 2 and 3 behavior.

The endojs is for secure platforms (as far as i understand), which it's design should not affect anonymous package's specific syntax.

  1. blocking dynamic import - agree - since dynamic import is mostly used on end user level (end developer)
  2. blocking member function named import - disagree - since class member can be named anything as it can be and not allowing this violates the concept of class.
  3. import keyword in comment - needs improvement

@kriskowal
Copy link
Member

kriskowal commented Jun 15, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants