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

Decode: fail when array is missing separator #616

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

jidicula
Copy link
Contributor

@jidicula jidicula commented Oct 5, 2021

Issue: #613

The toml-test generated test for an array with missing separators was
failing since the parser wasn't noticing missing separators.

This change leverages the zero value of ast.Reference (an int). The
var is now declared outside the while block, and re-zeroed if the
token is not a non-trailing comma.

The parser checks if the previous value of valueRef is zero. The
only time this check would pass is when the previous byte in the array
was a whitespace, which would only be the case when an array is
missing a separator.

Fixes go test -tags testsuite -run TestTOMLTest_Invalid_Array_MissingSeparator

@jidicula jidicula marked this pull request as draft October 5, 2021 18:58
@jidicula jidicula marked this pull request as ready for review October 6, 2021 00:39
@jidicula jidicula marked this pull request as draft October 6, 2021 01:20
@jidicula jidicula force-pushed the ji/issue-613-array-missing-separator branch from 983d3d9 to dc095fa Compare October 6, 2021 01:37
@jidicula jidicula marked this pull request as ready for review October 6, 2021 01:39
The toml-test generated test for an array with missing separators was
failing since the parser wasn't noticing missing separators.

This change leverages the zero value of `ast.Reference` (an int). The
var is now declared outside the while block, and re-zeroed if the
token is not a non-trailing comma.

The parser checks if the previous value of `valueRef` is zero. The
only time this check would pass is when the previous byte in the array
was a whitespace, which would only be the case when an array is
missing a separator.

Resolves: part of pelletier#613
@jidicula jidicula force-pushed the ji/issue-613-array-missing-separator branch from dc095fa to 5875df6 Compare October 8, 2021 20:03
@pelletier
Copy link
Owner

pelletier commented Oct 14, 2021

Sorry for the late reply! Thank you for the patch! Adjusted it to return directly when the failure is detected.

Note: the building is failing because CI does not run the testsuite tests, and don't get the coverage from those tests. I'll fix that in a PR.

@pelletier pelletier merged commit 86632bc into pelletier:v2 Oct 14, 2021
@jidicula jidicula deleted the ji/issue-613-array-missing-separator branch October 14, 2021 12:35
@jidicula
Copy link
Contributor Author

Sorry for the late reply! Thank you for the patch! Adjusted it to return directly when the failure is detected.

Good call, I should have seen that. Thanks!

@pelletier pelletier changed the title fix(array test): Fail with missing separator Decode: fail when array is missing separator Oct 28, 2021
@pelletier pelletier added the bug Issues describing a bug in go-toml. label Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants