nkuprins opened a new issue, #953:
URL: https://github.com/apache/fesod/issues/953

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/fesod/issues) and 
found nothing similar.
   
   
   ### Motivation
   
   `BooleanUtils.valueOf(String)` in `fesod-common` returns `TRUE` only for the 
exact string `"1"` - every other input, including `"true"`, maps to `FALSE`:
   
   ```java
   public static Boolean valueOf(String str) {
       if (TRUE_NUMBER.equals(str)) {  // TRUE_NUMBER = "1"
           return Boolean.TRUE;
       }
       return Boolean.FALSE;
   }
   ```
   
   Its only production caller is `CellTagHandler`, converting the `<v>` value 
of `t="b"` (boolean) cells when streaming xlsx. The strict `"1"` only check 
matches Excel's own output (Excel always writes `0`/`1`) and Apache POI's DOM 
model (`XSSFCell.getBooleanCellValue()` does the same `"1".equals(v)`), so it 
looks intentional.
   
   However, `BooleanUtilsTest` only covers `"1"`, `""`, and `null`. Nothing in 
the repo documents that `valueOf("true") == FALSE` is by design. Notably, POI's 
own SAX path (`XSSFSheetXMLHandler`) is more lenient (`value.charAt(0) == '0' ? 
FALSE : TRUE`), so the precedent is mixed and a future contributor could 
plausibly "fix" this as a bug and change reading behavior.
   
   **Question for the maintainers:** is the `"1"` only matching the intended 
contract? If yes, would you accept a small test-only PR pinning it?
   
   ### Solution
   
   If the current behavior is intended, add assertions to `BooleanUtilsTest` 
documenting it, e.g.:
   
   ```java
   // Intentional: xlsx boolean cells use Excel's 0/1 markers; matches POI 
XSSFCell
   Assertions.assertFalse(BooleanUtils.valueOf("true"));
   Assertions.assertFalse(BooleanUtils.valueOf("0"));
   ```
   
   optionally plus a one-line Javadoc note on `valueOf` stating it targets 
Excel's `0`/`1` boolean cell markers.
   
   ### Alternatives
   
   If instead the maintainers consider the strictness a defect (files written 
by non-Excel producers could in principle contain `<v>true</v>`), the 
alternative is making valueOf lenient - e.g. also accepting "true" 
case-insensitively, similar to POI's XSSFSheetXMLHandler. That would be a 
behavior change to xlsx boolean reading, so I did not assume it.
   
   ### Anything else?
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to