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

Remove unnecessary string escaping #67

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mschwager
Copy link
Member

There are two benefits here:

  1. Avoiding the need for escaping makes things more clear.
  2. This form of backslash escaping was actually only added in YAML 1.2.0. Some YAML parsers are not fully compliant with YAML 1.2.0, so they fail to parse these rules. This isn't a problem for Semgrep, but can be an issue for other tools or libraries analyzing these rules.

@mschwager
Copy link
Member Author

Actually, it appears double quotes works without the escaping: 54a018a.

I'm not sure why I thought these strings needed escaped forward slashes in the first place. Apparently forward slashes do not need to be escaped, and this was only added in YAML 1.2.0 to be backwards compatible with JSON. However, even JSON does not require forward slashes to be escaped, and the original reason for allowing it was something to do with JavaScript code 🤷.

@mschwager mschwager changed the title Use single quotes for strings to avoid escaping Remove unnecessary string escaping Oct 15, 2024
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.

1 participant