Skip to content

Tests for Newtonsoft.Json's handling of octal literals in version files#4227

Merged
HebaruSan merged 1 commit into
KSP-CKAN:masterfrom
HebaruSan:add/avc-json-octal-tests
Oct 9, 2024
Merged

Tests for Newtonsoft.Json's handling of octal literals in version files#4227
HebaruSan merged 1 commit into
KSP-CKAN:masterfrom
HebaruSan:add/avc-json-octal-tests

Conversation

@HebaruSan
Copy link
Copy Markdown
Member

@HebaruSan HebaruSan commented Oct 9, 2024

Motivation

A mod uploaded a release today with this in its version file:

		"PATCH":09,

This causes an inflation error:

New inflation error for <mod name>: Error parsing version file <path to version file>.version: Input string '09' is not a valid number. Path 'VERSION.PATCH', line 9, position 12.

Digging into this, apparently a 0 prefix for numbers isn't technically allowed at all in strict JSON (and a few online validators confirm this), but some parsers (including the most popular one for C#, Newtonsoft.Json) decided to be lenient and parse them as C-style octal literals. But 08 and 09 are not valid octal literals because only the digits 01234567 can be used in octal.

Changes

Now several new test cases are added to check and document exactly what is and is not allowed by Newtonsoft.Json's numeric parser, for future reference.

Note that the QuotedInvalidOctalNumber test is technically using an exploit, as the version file schema requires these fields to be integers, not strings, but CKAN does not enforce this.

@HebaruSan HebaruSan added Easy This is easy to fix Netkan Issues affecting the netkan data Tests Issues affecting the internal tests labels Oct 9, 2024
@JonnyOThan
Copy link
Copy Markdown
Contributor

for reference here is the official json grammar: https://www.json.org/json-en.html

@HebaruSan HebaruSan merged commit 8c02754 into KSP-CKAN:master Oct 9, 2024
@HebaruSan HebaruSan deleted the add/avc-json-octal-tests branch October 9, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Easy This is easy to fix Netkan Issues affecting the netkan data Tests Issues affecting the internal tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants