lukaszlenart opened a new pull request, #1824: URL: https://github.com/apache/struts/pull/1824
Fixes [WW-5668](https://issues.apache.org/jira/browse/WW-5668) Follow-up to #1821, which is already merged. Two defects in the serialization handling that change shipped, both only observable when deserializing a stream written by an **earlier** release — which is exactly what a rolling upgrade does. ## 1. `serialVersionUID = 1L` breaks upgrades from 7.2.1 The localized-text providers are reachable from the `ValueStack` and really are serialized — that is why #1821 had to make the caches `transient` in the first place. Where a session is replicated or persisted, a 7.2.1 node's stream can be read by a 7.3.0 node. 7.2.1 declared no explicit `serialVersionUID`, so its streams carry the value the JVM computed from that class shape. Pinning `1L` does not match it, and deserialization fails: ``` java.io.InvalidClassException: org.apache.struts2.text.AbstractLocalizedTextProvider; local class incompatible: stream classdesc serialVersionUID = -4455624669971032217, local class serialVersionUID = 1 ``` This pins each provider to the value `serialver` reports for the `STRUTS_7_2_1` tag instead: | Class | Value | | ----- | ----- | | `AbstractLocalizedTextProvider` | `-4455624669971032217L` | | `StrutsLocalizedTextProvider` | `1939638936989370989L` | | `GlobalLocalizedTextProvider` | `3777960740495792359L` | Accepting the older stream is safe precisely *because* #1821 made the caches `transient`: the stream's stale cache contents no longer match a field in the class, so they are discarded, and `readObject` rebuilds the caches empty. ## 2. `readObject` throws NPE on a stream without the new cache settings `i18nCacheType` and `i18nCacheMaxSize` are non-transient fields with initialisers. Field initialisers do not run during deserialization, and a 7.2.1 stream carries no value for them, so `defaultReadObject` leaves them `null`/`0` and `rebuildI18nCaches()` fails: ``` java.lang.NullPointerException: Cannot invoke "CacheType.ordinal()" because "cacheType" is null at AbstractLocalizedTextProvider.rebuildI18nCaches(...) at AbstractLocalizedTextProvider.readObject(...) ``` On merged `main` this is masked by the `InvalidClassException` above, so fixing only item 1 would have exposed it. `readObject` now restores the defaults before rebuilding. The existing `testProviderIsUsableAfterDeserialization` could not catch this: it round-trips a stream written by the *current* class, which does carry both fields. ## Testing - New `testProviderIsUsableAfterDeserializingLegacyStream` reproduces the absent-field state and fails with the exact NPE above when the guard is removed. - Verified end to end outside the suite: a provider serialized against a build of the `STRUTS_7_2_1` tag now deserializes cleanly against this branch, and fails with the `InvalidClassException` above against `main` as merged. - Full `core` suite green: 3149 tests, 0 failures, 0 errors. ## Backward compatibility Restores compatibility that #1821 removed. No API or behavioural change; a same-version round trip is unaffected, since both settings are still serialized normally. The equivalent fix for the 6.x line is in #1823. 🤖 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]
