pjfanning opened a new pull request, #1240:
URL: https://github.com/apache/poi/pull/1240
Three defects found while reviewing poi-ooxml. All are independent and small.
### `ContentType.equals()` is inverted
```java
return (!(obj instanceof ContentType))
|| (StringUtil.equalsIgnoreCase(this.toString(), obj.toString()));
```
This returns **true** for every argument that is not a `ContentType`,
including unrelated types and `String`. Fixed to require a `ContentType`.
While fixing it: `equals()` compares `toString()` case-insensitively, but
`hashCode()` was `Objects.hash(type, subType, parameters)`, which is
case-sensitive. Two content types differing only in case therefore compared
equal but hashed differently, violating the `equals`/`hashCode` contract. That
matters here because `ContentType` is a live `HashMap` key —
`OPCPackage.partMarshallers` and `partUnmarshallers` are looked up per part on
the open and save paths (`ZipPackage:701`, `OPCPackage:968`), so a content type
declared with different casing could miss its marshaller. RFC 2616 media types
are case-insensitive, so `hashCode()` is now derived from the same
case-insensitive form `equals()` uses.
Added `TestContentType.testEqualsAndHashCode` covering both the
case-insensitive equality and the non-`ContentType` argument.
### `PackagePartName` [M1.10] check never tested what it claimed
```java
if (seg.replaceAll("\\\\.", "").isEmpty()) {
```
In a Java string literal that regex is `\\.`, which matches a literal
backslash followed by any character — not dots. The rule ("a segment shall
include at least one non-dot character") was therefore never enforced by this
code. Replaced with an explicit all-dots check, which is also allocation-free.
Note this branch stays effectively unreachable in practice, since the
preceding [M1.9] check already rejects any segment ending in a dot, and an
all-dot segment always does — the existing comment says as much. The fix makes
the check correct rather than changing observable behaviour.
### `XSSFRow.removeCell` continues scanning after removal
The loop iterates a snapshot from `getCArray()` and calls `_row.removeC(i)`,
which shifts the underlying list, but then keeps scanning with a now-misaligned
`i`. Only one entry can match by object identity, so this does not misbehave
today, but the loop should stop at the match.
`poi-ooxml` OPC tests (183) plus `TestXSSFRow` pass locally.
🤖 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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
