dictParser rejects input files that lack a trailing newline - #232
Open
NRavoisin96 wants to merge 1 commit into
Open
dictParser rejects input files that lack a trailing newline#232NRavoisin96 wants to merge 1 commit into
NRavoisin96 wants to merge 1 commit into
Conversation
The scan loop in parseDict starts an iteration only while pos is strictly before the end of the tape, so the last character is never a valid starting position. Any input whose final character is the closing brace therefore ends with that brace unparsed, and the caller reports that the string was not fully read. charToDict has been working around this by appending ' }' instead of '}', where the space serves only to enforce the closing brace being processed; fileToDict has no such padding, so a file whose last byte is '}' fails outright. Fix the loop to include the final position, and drop the padding from the append. Add a test for a file without a trailing newline and one for character input ending with a brace, both of which produce the '}}' ending the padding used to avoid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mechanism
dictParser_func.f90parses a file by streaming its contents into acharTape(readFileContents, which converts eachend-of-record to a space—noted in the code itself at :74), appending the top-level dictionary terminator (:162-163):
! Append with dictionary terminator call file % append('}')and walking the tape with (:273):
For a file that ends with a newline, the converted trailing space sits between the last token and the appended
'}', so the loop condition still holds when the terminator is scanned. For a valid input file whose last line has no trailing newline, the appended'}'lands at exactlytape % length()with the previous token immediately before it: after that token is consumed,pos == length, the strict<exits the loop with the terminator unconsumed, and the parse aborts with:The same off-by-one is why
charToDictcarries an explicit workaround (:223-224):with the leading space existing only to keep the terminator off the exact end of the tape when the caller's data itself ends in
'}'.Impact
Any input file whose final line lacks a newline is affected. The abort fires on an otherwise valid command-line invocation
(
scone.f90:50feeds the path straight tofileToDict), with an error message pointing at a missing'}'that is, in fact, present.Provenance
Present since
ed517f67("New dictionary parser", 2020-03-14).Reproduction
fileToDictonf.txt(e.g., via./Build/scone.out f.txt) aborts with the message above.Fix
Two small changes. First, fix the loop bound to include tape % length when parsing:
and, in
charToDict, the workaround is removed to matchfileToDict:! Append with dictionary terminator. call file % append('}')Note that this only makes the final position a valid starting position for the scan; it does not weaken any termination check. If an inner dictionary is genuinely unterminated, it consumes the top-level terminator early and the outer parse still reaches the same fatalError; a
'{'at the tape end still errors in the recursive call.Test plan
First, a new integration test file,
IntegrationTestFiles/testDictionaryNoNewline, whose content ends in'}'with no trailing newline (git records and preserves the missing end-of-line).Second, two new tests attached to the PR:
testFromFileNoTrailingNewlineindictParser_iTest.f90asserting the above file parses. On currentmainthis test aborts with the error message above.charToDictcase with data ending exactly in'}'indictParser_test.f90that explicitly tests the workaround removal. (Note: the existingtestFromChartape also ends in'}'and doubles as a regression for this path.)