[
https://issues.apache.org/jira/browse/WW-5685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5685:
------------------------------
Description:
h3. Problem
{{DefaultConversionFileProcessor.process}} iterates the entries of a
{{-conversion.properties}} file and skips keys that are already present in the
converter mapping. The skip is written as a {{break}} rather than a
{{continue}}, so instead of skipping that one entry it abandons the whole file:
{code:java}
for (Map.Entry entry : prop.entrySet()) {
String key = (String) entry.getKey();
if (mapping.containsKey(key)) {
break; // <-- should be continue
}
...
}
{code}
Every remaining entry in that properties file is silently dropped. There is no
warning and no error; the affected properties simply fall back to default
conversion at runtime.
h3. Why it triggers in practice
{{XWorkConverter.buildConverterMapping}} walks the class hierarchy — the class
itself, then its interfaces, then its superclass — and passes _one shared,
accumulating_ mapping into each {{addConverterMapping}} call. So a key claimed
earlier in that walk aborts a later file entirely:
* A subclass {{Foo-conversion.properties}} declares {{bar}}.
* The superclass {{FooBase-conversion.properties}} declares {{bar}}, {{baz}}
and {{qux}}.
* When the superclass file is processed, {{bar}} is already mapped, so {{baz}}
and {{qux}} are never registered.
Annotation-derived entries land in the same map, so an {{@TypeConversion}} on
the class can abort its properties file the same way.
h3. Nondeterminism
{{Properties}} extends {{Hashtable}} and {{entrySet()}} has no defined
iteration order, so _which_ entries survive depends on hash order rather than
file order. The same file can behave differently across JDK versions or after
an unrelated key is added, which makes this hard to diagnose from the symptom.
h3. Related
WW-3871 fixed the identical {{break}} instead of {{continue}} defect in the
annotation path ({{DefaultConversionAnnotationProcessor}}), merged as PR #1812
and shipped in 7.3.0. This is the same bug in the properties-file path, which
that change did not touch. It was noted during that work but deliberately left
out of scope.
h3. Suggested fix
Change the {{break}} to {{continue}}, and add a regression test with a
{{-conversion.properties}} file whose first key is already mapped, asserting
the later keys still register. A hierarchy case (subclass and superclass files
sharing a key) covers the realistic trigger.
was:
h3. Problem
{{DefaultConversionFileProcessor.process}} iterates the entries of a
{{-conversion.properties}} file and skips keys that are already present in the
converter mapping. The skip is written as a {{break}} rather than a
{{continue}}, so instead of skipping that one entry it abandons the whole file:
{code:java}
for (Map.Entry entry : prop.entrySet()) {
String key = (String) entry.getKey();
if (mapping.containsKey(key)) {
break; // <-- should be continue
}
...
}
{code}
Every remaining entry in that properties file is silently dropped. There is no
warning and no error; the affected properties simply fall back to default
conversion at runtime.
h3. Why it triggers in practice
{{XWorkConverter.buildConverterMapping}} walks the class hierarchy — the class
itself, then its interfaces, then its superclass — and passes _one shared,
accumulating_ mapping into each {{addConverterMapping}} call. So a key claimed
earlier in that walk aborts a later file entirely:
* A subclass {{Foo-conversion.properties}} declares {{bar}}.
* The superclass {{FooBase-conversion.properties}} declares {{bar}}, {{baz}}
and {{qux}}.
* When the superclass file is processed, {{bar}} is already mapped, so {{baz}}
and {{qux}} are never registered.
Annotation-derived entries land in the same map, so an {{@TypeConversion}} on
the class can abort its properties file the same way.
h3. Nondeterminism
{{Properties}} extends {{Hashtable}} and {{entrySet()}} has no defined
iteration order, so _which_ entries survive depends on hash order rather than
file order. The same file can behave differently across JDK versions or after
an unrelated key is added, which makes this hard to diagnose from the symptom.
h3. Related
WW-3871 fixed the identical {{break}}-instead-of-{{continue}} defect in the
annotation path ({{DefaultConversionAnnotationProcessor}}), merged as PR #1812
and shipped in 7.3.0. This is the same bug in the properties-file path, which
that change did not touch. It was noted during that work but deliberately left
out of scope.
h3. Suggested fix
Change the {{break}} to {{continue}}, and add a regression test with a
{{-conversion.properties}} file whose first key is already mapped, asserting
the later keys still register. A hierarchy case (subclass and superclass files
sharing a key) covers the realistic trigger.
> DefaultConversionFileProcessor silently drops the rest of a
> -conversion.properties file after the first already-mapped key
> --------------------------------------------------------------------------------------------------------------------------
>
> Key: WW-5685
> URL: https://issues.apache.org/jira/browse/WW-5685
> Project: Struts 2
> Issue Type: Bug
> Components: Core Actions
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
>
> h3. Problem
> {{DefaultConversionFileProcessor.process}} iterates the entries of a
> {{-conversion.properties}} file and skips keys that are already present in
> the converter mapping. The skip is written as a {{break}} rather than a
> {{continue}}, so instead of skipping that one entry it abandons the whole
> file:
> {code:java}
> for (Map.Entry entry : prop.entrySet()) {
> String key = (String) entry.getKey();
> if (mapping.containsKey(key)) {
> break; // <-- should be continue
> }
> ...
> }
> {code}
> Every remaining entry in that properties file is silently dropped. There is
> no warning and no error; the affected properties simply fall back to default
> conversion at runtime.
> h3. Why it triggers in practice
> {{XWorkConverter.buildConverterMapping}} walks the class hierarchy — the
> class itself, then its interfaces, then its superclass — and passes _one
> shared, accumulating_ mapping into each {{addConverterMapping}} call. So a
> key claimed earlier in that walk aborts a later file entirely:
> * A subclass {{Foo-conversion.properties}} declares {{bar}}.
> * The superclass {{FooBase-conversion.properties}} declares {{bar}}, {{baz}}
> and {{qux}}.
> * When the superclass file is processed, {{bar}} is already mapped, so
> {{baz}} and {{qux}} are never registered.
> Annotation-derived entries land in the same map, so an {{@TypeConversion}} on
> the class can abort its properties file the same way.
> h3. Nondeterminism
> {{Properties}} extends {{Hashtable}} and {{entrySet()}} has no defined
> iteration order, so _which_ entries survive depends on hash order rather than
> file order. The same file can behave differently across JDK versions or after
> an unrelated key is added, which makes this hard to diagnose from the symptom.
> h3. Related
> WW-3871 fixed the identical {{break}} instead of {{continue}} defect in the
> annotation path ({{DefaultConversionAnnotationProcessor}}), merged as PR
> #1812 and shipped in 7.3.0. This is the same bug in the properties-file path,
> which that change did not touch. It was noted during that work but
> deliberately left out of scope.
> h3. Suggested fix
> Change the {{break}} to {{continue}}, and add a regression test with a
> {{-conversion.properties}} file whose first key is already mapped, asserting
> the later keys still register. A hierarchy case (subclass and superclass
> files sharing a key) covers the realistic trigger.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
