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

Hashes - Fix JSON parsing function of numerals formatted as scientific notation with uppercase E #1580

Merged
merged 2 commits into from
Sep 3, 2023

Conversation

initramfs
Copy link
Contributor

@initramfs initramfs commented Jun 28, 2023

On both the json.org site and the relevant ECMA-404 specification a number symbol is defined as containing an exponent symbol which may be: empty, "e" (U+0065, LATIN SMALL LETTER E), or "E" (U+0045, LATIN CAPITAL LETTER E).

The current parsing code will error on numbers using the capital "E" syntax such as 1.2E5.

When merged this pull request will:

Implement the ability to parse numbers in scientific notation using capital "E" by adding the relevant token to the _numeric variable. The SQF script command parseNumber already handles numbers using uppercase "E" as an exponent delimiter without issue so no changes are required elsewhere (i.e. we do not need to casefold the lexed numeric token prior to feeding into the scripting engine).

@jonpas jonpas added this to the 3.15.9 milestone Jun 28, 2023
@jonpas jonpas added the Bug Fix label Jun 28, 2023
@jonpas
Copy link
Member

jonpas commented Jun 28, 2023

@BaerMitUmlaut

@BaerMitUmlaut
Copy link
Contributor

Since we now have regex, this could be improved further for more edge cases.

@initramfs
Copy link
Contributor Author

initramfs commented Jun 28, 2023

@BaerMitUmlaut I can modify the tokenizer to use a regex to validate the number parsed, but what should it do if an invalid numeric token is found? The rest of the code seems to do no validation whatsoever and just assumes the JSON is valid (e.g here).

If the intent is for an invalid JSON document to crash the script, is regex parsing necessary (since parseNumber would likely complain anyway)?

@BaerMitUmlaut
Copy link
Contributor

Good point. Let's leave it like this (with your change) unless we add proper error handling.

@jonpas jonpas merged commit 5b66bbf into CBATeam:master Sep 3, 2023
4 checks passed
@jonpas jonpas changed the title Fix JSON parsing function of numerals formatted as scientific notation with uppercase E Hashes - Fix JSON parsing function of numerals formatted as scientific notation with uppercase E Sep 3, 2023
@initramfs initramfs deleted the fix-parse-json branch September 3, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants