gnodet-bot commented on code in PR #408:
URL: https://github.com/apache/maven-filtering/pull/408#discussion_r4105034433
##########
src/main/java/org/apache/maven/shared/filtering/InterpolatorFilterReaderLineEnding.java:
##########
@@ -309,6 +317,8 @@ public int read() throws IOException {
if (value != null) {
replaceData = value;
replaceIndex = value.length();
+ } else if (failOnMissingFilterValue) {
+ throw new IOException("Unresolved filter token: '" + key + "'");
Review Comment:
💡 **Observation:** When `end != 0` (begin token found but end token not
matched before EOF/EOL — e.g. an unterminated `${unclosed` at end of file),
`value` stays `null` because interpolation is skipped (`if (end == 0)` guard
above). This check then fires and throws, even though the token isn't genuinely
"unresolved" — it's malformed/unterminated.
The `MultiDelimiterInterpolatorFilterReaderLineEnding` doesn't have this
issue because it returns early on `end != 0` (`in.reset(); return in.read();`)
before reaching this code.
Arguably, failing on unterminated expressions in strict mode is reasonable
behavior (the user opted into strictness, and an unterminated expression is
suspicious). But if you want to match the existing pass-through semantics for
malformed tokens specifically, you could guard with:
```suggestion
} else if (failOnMissingFilterValue && end == 0) {
throw new IOException("Unresolved filter token: '" + key + "'");
```
This restricts the check to tokens where the end delimiter was actually
found but interpolation returned null — i.e., genuinely unresolvable values.
Not blocking — just flagging the behavioral difference between the two reader
implementations.
--
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]
