Jens-G opened a new pull request, #3840: URL: https://github.com/apache/thrift/pull/3840
Implements [THRIFT-6212](https://issues.apache.org/jira/browse/THRIFT-6212). ## The defect If the last comment in a `.thrift` file is never closed, the compiler loops forever instead of reporting an error: ``` $ printf '/* unterminated comment' > c.thrift $ thrift --gen cpp c.thrift # never returns $ printf '/** unterminated doc' > d.thrift $ thrift --gen cpp d.thrift # never returns ``` Both comment rules in `compiler/cpp/src/thrift/thriftl.ll`, `doctext_begin` and `multicm_begin`, compare the result of `yyinput()` with `EOF`. The scanner flex generates never returns `EOF` from `yyinput()`. At end of input it returns `0`, and it keeps returning `0` on every later call. So `case EOF:` never matches: the `0` falls through to `default:`, a NUL byte is appended to `parsed`, and the loop reads again. It never stops, and `parsed` keeps growing. The input can also come from a memory buffer instead of a file. That is how the compiler unit tests drive the lexer, via `yy_scan_bytes()`. There is no `yyin` in that case, so the loop does not spin. It crashes in `fread()` on a null `FILE*`. ## The change THRIFT-4244 (#3786) fixed the same defect in the string-literal scanner. There, `yywrap()` sets `yyinput_reached_eof`, which separates a real end of input from a literal NUL byte. Both comment scanners now check that flag. `case EOF:` becomes `case 0:`. If the flag is set, the scanner reports `Unexpected end of file in doc-comment at <line>` (or `... in multiline comment at <line>`) and exits. Otherwise the byte is ordinary comment content, as before. The fix checks the flag instead of treating every `0` as end of input. That way a literal NUL inside a closed comment keeps its current behaviour. Any input that terminates today takes the same path as before. The change only affects input that used to loop forever. ## Tests `compiler/cpp/tests/cpp/t_cpp_parser_comment_tests.cc` follows the string tests from #3786: - `lexer diagnoses an unterminated comment at end of file`: `/* ...` and `/** ...` at end of input throw the diagnostic. - `lexer keeps reading a comment after a literal NUL`: a NUL inside a closed comment of either kind is still accepted. This test also passes on master. It is there so the fix cannot treat every `0` as end of input. Both were run in `thrift:jammy`, built the way the *Build with CMake* workflow builds (`cmake -DBUILD_LIBRARIES=OFF`, then `ctest`): | | master's lexer | this change | |---|---|---| | `lexer diagnoses an unterminated comment at end of file` | crashes (`SIGSEGV`) | pass | | `lexer keeps reading a comment after a literal NUL` | pass | pass | | whole `thrift_compiler_tests` binary | crashes (`SIGSEGV`) | 330 assertions in 24 test cases pass, also with `--order rand` | | `ctest` | not run | 18 of 18 pass | | `thrift --gen cpp c.thrift` / `d.thrift` | still running when a 10 s timeout killed it | exits 1 with the diagnostic | Accepted input is unaffected. Every generator (29 of them) was run on `tutorial.thrift`, `shared.thrift`, `ThriftTest.thrift` and `DocTest.thrift`, once with a build of unpatched master and once with this change. The generated files (909 of them), stderr and exit codes were identical across all 116 runs. That includes 9 runs that fail on both builds, because those generators do not support a type that `ThriftTest.thrift` uses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
