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

Add adjusted test and new auto fixer for 'no-insecure-url' #39

Merged
merged 2 commits into from Jun 14, 2023

Conversation

moztomer
Copy link
Contributor

Goal of this patch

  • introducing a --fix option for no-insecure-url (only for http:// but could be extended to fix also ftp and ws)
  • introducing an exception list for vairable names

Changes

  • removed one duplicate in DEFAUL_EXCEPTIONS
  • lines for --fix option:
    There are two fixers. The first for the Literals is escaping the string, replaces http by https and write it back in the related line.
   fix(fixer) {
      // Only fix if it contains an http url
      if (node.value.toLowerCase().includes("http")) {
        let fixedString = node.value.replace(/http:/i, "https:");
        //insert an "s" before ":/" to change http:/ to https:/
        return fixer.replaceText(node, JSON.stringify(fixedString));
      }
    },

The second one is for TemplateElements. It escapes the string, then removes the "" that JSON.stringify created, replace http by https in the correct manner (in backticks) and write it back into the related line.

 fix(fixer) {
  // Only fix if it contains an http url
  if (node.value.raw.toLowerCase().includes("http")) {
      let escapedString =  JSON.stringify(context.getSourceCode().getText(node));
      // delete "" that JSON.stringify created and convert to `` string
      escapedString = ``+ escapedString.substring(1, escapedString.length-1);
      let fixedString = escapedString.replace(/http:/i, "https:");
      //insert an "s" before ":/" to change http:/ to https:/
      return fixer.replaceText(node, fixedString);
    }
  }
  • introduce shoudlFix() function
        function shouldFix( varExceptions,context, node) {
          let sourceCode = context.getSourceCode();
          // check variable for unfixable pattern e.g. `var insecureURL = "http://..."`
          let text = node.parent
            ? sourceCode.getText(node.parent)
            : sourceCode.getText(node);
          // if no match, fix the line
          return !matches(varExceptions,text);
        }

which checks if a line should get exempted from linter because it includes a variable name that shouldn't get upgraded.

  • added test for auto-fixer (string escaping, backtick strings and both combined)
  • added output: to each test that gets affected by auto-fixer
  • added test for the new introduced variable exception list for variables
  • adjusted README file of no-insecure-url to include new exception list

@moztomer
Copy link
Contributor Author

@A-Katopodis @Vflouirac Hi folks,
Do you think you could benefit from a --fix option for your no-insecure-url rule? :)

@moztomer
Copy link
Contributor Author

@A-Katopodis @Vflouirac
Is there interest from your side in such a patch? :)

@mozfreddyb
Copy link
Contributor

@Vflouirac This pull request was approved many months ago. Any chance this can land? @moztomer and I are in scope of the existing CLA between Mozilla and Microsoft (and the check passed!).

@Vflouirac Vflouirac merged commit 31d6d68 into microsoft:main Jun 14, 2023
3 checks passed
@Vflouirac
Copy link
Contributor

Vflouirac commented Jun 14, 2023 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

Successfully merging this pull request may close these issues.

None yet

3 participants