You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fesod.apache.org by "roeniss (via GitHub)" <gi...@apache.org> on 2026/01/18 13:21:42 UTC
[PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
roeniss opened a new pull request, #813:
URL: https://github.com/apache/fesod/pull/813
<!--
Thanks very much for contributing to Apache Fesod (Incubating)! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
-->
## Purpose of the pull request
Closed: #462
## What's changed?
Added a bounds check after the inner while loop to break out when no valid (non-escaped) closing brace is found.
## Checklist
- [x] I have read the [Contributor Guide](https://fesod.apache.org/community/contribution/).
- [ ] I have written the necessary doc or comment. (in this case, no need I guess)
- [x] I have added the necessary unit tests and all cases have passed.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "Mikkey-f (via GitHub)" <gi...@apache.org>.
Mikkey-f commented on PR #813:
URL: https://github.com/apache/fesod/pull/813#issuecomment-6120156650
Hi @roeniss — checking in on this one. psxjoy mentioned back in April that it would be good to merge once the branch is updated and the code moved into the right place, and the PR has been quiet since July.
Are you still planning to carry it forward? If you don't have the bandwidth for it right now, I'd be happy to pick it up — I'd re-apply the `suffixIndex < 0` guard on current main, address psxjoy's placement note with a test that follows the current testkit conventions, and reference this PR and #462 in the description so the original diagnosis stays credited. Either way, just let me know.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "psxjoy (via GitHub)" <gi...@apache.org>.
psxjoy commented on PR #813:
URL: https://github.com/apache/fesod/pull/813#issuecomment-4174417029
@roeniss Hi, can you update the branch and let code in right place ? I think we can merge this PR if everything is already.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "Mikkey-f (via GitHub)" <gi...@apache.org>.
Mikkey-f commented on PR #813:
URL: https://github.com/apache/fesod/pull/813#issuecomment-6139842376
@roeniss Thanks for the go-ahead — carrying it forward as promised.
Re-applied the `suffixIndex < 0` guard on current main (your commit is kept,
with you as the author) and rewrote the test against the current testkit
conventions: the `TestFileUtil` it used was removed in #901, so the test now
extends `AbstractExcelTest` and builds its templates at runtime. Your four
scenarios are preserved, and the red-side evidence matches the original failure
exactly (`StringIndexOutOfBoundsException: begin 1, end -1, length 6`).
New PR: #1218 (credits this PR and #462 in its description). Once it is merged,
this one can be closed — thanks again for the diagnosis!
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #813:
URL: https://github.com/apache/fesod/pull/813#discussion_r2835921623
##########
fesod-examples/fesod-sheet-examples/src/test/java/org/apache/fesod/sheet/temp/issue462/EscapedPlaceholderTest.java:
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.temp.issue462;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.fesod.sheet.FesodSheet;
+import org.apache.fesod.sheet.util.TestFileUtil;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case for issue #462: ArrayIndexOutOfBoundsException when placeholder content is '{foo\}'
+ *
+ * When a placeholder has an escaped closing brace like {foo\}, the parser should handle it
+ * gracefully without throwing an exception.
+ */
+public class EscapedPlaceholderTest {
+
+ private static final String TEMPLATE_PATH = TestFileUtil.getPath() + "temp/issue462" + File.separator;
+
+ @BeforeAll
+ public static void createTemplates() throws IOException {
+ // Create template with escaped placeholder {foo\}
+ createTemplate("escaped_suffix.xlsx", "{foo\\}");
+
+ // Create template with escaped prefix \{foo}
+ createTemplate("escaped_prefix.xlsx", "\\{foo}");
+
+ // Create template with normal placeholder for comparison
+ createTemplate("normal.xlsx", "{foo}");
+
+ // Create template with mixed content
+ createTemplate("mixed.xlsx", "prefix {foo\\} suffix");
+ }
+
+ private static void createTemplate(String fileName, String content) throws IOException {
+ File templateFile = new File(TEMPLATE_PATH + fileName);
Review Comment:
`createTemplate` writes to `TEMPLATE_PATH + fileName`, but it never ensures that `temp/issue462/` exists under the test classpath root. Since `src/test/resources/temp/` exists but `temp/issue462/` does not, `new FileOutputStream(templateFile)` will throw `FileNotFoundException` on a clean checkout. Create the parent directories before opening the stream (or reuse `TestFileUtil.createNewFile(...)` which already creates parent dirs).
```suggestion
File templateFile = new File(TEMPLATE_PATH + fileName);
File parentDir = templateFile.getParentFile();
if (parentDir != null && !parentDir.exists() && !parentDir.mkdirs()) {
throw new IOException("Failed to create directory: " + parentDir);
}
```
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "roeniss (via GitHub)" <gi...@apache.org>.
roeniss commented on code in PR #813:
URL: https://github.com/apache/fesod/pull/813#discussion_r2837208078
##########
fesod-examples/fesod-sheet-examples/src/test/java/org/apache/fesod/sheet/temp/issue462/EscapedPlaceholderTest.java:
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.temp.issue462;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.fesod.sheet.FesodSheet;
+import org.apache.fesod.sheet.util.TestFileUtil;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case for issue #462: ArrayIndexOutOfBoundsException when placeholder content is '{foo\}'
+ *
+ * When a placeholder has an escaped closing brace like {foo\}, the parser should handle it
+ * gracefully without throwing an exception.
+ */
+public class EscapedPlaceholderTest {
+
+ private static final String TEMPLATE_PATH = TestFileUtil.getPath() + "temp/issue462" + File.separator;
+
+ @BeforeAll
+ public static void createTemplates() throws IOException {
+ // Create template with escaped placeholder {foo\}
+ createTemplate("escaped_suffix.xlsx", "{foo\\}");
+
+ // Create template with escaped prefix \{foo}
+ createTemplate("escaped_prefix.xlsx", "\\{foo}");
+
+ // Create template with normal placeholder for comparison
+ createTemplate("normal.xlsx", "{foo}");
+
+ // Create template with mixed content
+ createTemplate("mixed.xlsx", "prefix {foo\\} suffix");
+ }
+
+ private static void createTemplate(String fileName, String content) throws IOException {
+ File templateFile = new File(TEMPLATE_PATH + fileName);
Review Comment:
@psxjoy I think this feedback doesn't make sense, what do you think?
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "roeniss (via GitHub)" <gi...@apache.org>.
roeniss commented on PR #813:
URL: https://github.com/apache/fesod/pull/813#issuecomment-6136261256
@Mikkey-f Hi Mikkey, you can carry the issue instead of me. Thanks!
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "roeniss (via GitHub)" <gi...@apache.org>.
roeniss commented on PR #813:
URL: https://github.com/apache/fesod/pull/813#issuecomment-3796836592
@psxjoy could you review this?
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: handle escaped closing brace in template placeholder parsing [fesod]
Posted by "roeniss (via GitHub)" <gi...@apache.org>.
roeniss commented on PR #813:
URL: https://github.com/apache/fesod/pull/813#issuecomment-3765447264
commit force-pushed due to spotless check
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org