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

Reject potentially unsafe characters in .aleo and .leo files #4

Merged
merged 4 commits into from
Oct 25, 2022

Conversation

acoglio
Copy link
Collaborator

@acoglio acoglio commented Oct 8, 2022

Mirrors https://github.com/AleoHQ/snarkVM/pull/1044, for Aleo instructions.

Mirrors Aleo instructions for Leo, but we still need to modify the Leo parser (actually lexer) for this.

Reformulate some rules to exclude potentially unsafe ASCII and non-ASCII
characters. Matches the current Aleo instruction parser (see
https://github.com/AleoHQ/snarkVM/pull/1044).
@acoglio acoglio changed the title Capture rejection of potentially unsafe characters in .aleo files Reject potentially unsafe characters in .aleo files Oct 9, 2022
Reformulate some rules to exclude potentially unsafe ASCII characters. The
exclusion is the same as in Aleo instructions.
@acoglio acoglio changed the title Reject potentially unsafe characters in .aleo files Reject potentially unsafe characters in .aleo and .leo files Oct 9, 2022
Copy link
Contributor

@bendyarm bendyarm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double-checked all the details and I think it is all correct.
The change is a good one. There is no reason to allow the nonprintable ASCII control characters in the source code's language.
One suggestion I have is to add comments near the beginning of the file that (1) link to the ABNF spec and (2) explain that the source code files must be in valid UTF-8 which is first decoded to code points before it is parsed with this grammar. For example, see the first two comments in this file: https://github.com/toml-lang/toml/blob/main/toml.abnf

@acoglio
Copy link
Collaborator Author

acoglio commented Oct 24, 2022

@bendyarm Adding that kind of comment seems like a good idea. I've just opened an Issue for that.

@acoglio acoglio merged commit dd324f7 into master Oct 25, 2022
@acoglio acoglio deleted the safe-chars branch October 25, 2022 16:25
acoglio added a commit that referenced this pull request Jan 5, 2023
Add references to ABNF notation.

Specify that the grammar applies to UTF-8-decoded files.

Closes #4.
@acoglio acoglio mentioned this pull request Jan 5, 2023
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