sbglasius opened a new pull request, #16296:
URL: https://github.com/apache/grails-core/pull/16296
Fixes #16294
Fixes #16295
`someObject as JSON` (and `as XML`) failed for two very common shapes of
object. Both are fixed here, since fixing the first one only exposes the second.
## #16294 — `IllegalAccessException` on a non-public class
A class that is not public — anonymous, local or package-private — cannot
have its read methods invoked reflectively from another package, even though
the methods themselves carry the `public` modifier. All four bean marshallers
called `readMethod.invoke(...)` with no accessibility handling, so handing `as
JSON` an anonymous implementation of a public interface (the usual shape of a
Spring Security `UserDetails`) failed with:
```
java.lang.IllegalAccessException: class
org.grails.web.converters.marshaller.json.GroovyBeanMarshaller
cannot access a member of class com.example.DemoController$1 with
modifiers "public"
```
The read method is now resolved to the interface method where one exists,
and made accessible otherwise:
```java
Method invokableMethod = ClassUtils.getInterfaceMethodIfPossible(readMethod,
clazz);
ReflectionUtils.makeAccessible(invokableMethod);
Object value = invokableMethod.invoke(o, (Object[]) null);
```
Resolving to the interface is not sufficient on its own — a package-private
class with a public getter and no interface has nowhere to resolve to — so
`makeAccessible` carries that case.
The public *field* loops of both `GroovyBeanMarshaller`s failed identically
on `field.get(o)` and are now made accessible too.
## #16295 — `IllegalArgumentException` on any `Serializable` bean
`GenericJavaBeanMarshaller` evaluated `field.canAccess(o)` before the static
check, and per its javadoc `canAccess` throws for a static member when the
object is non-null. Any bean declaring `private static final long
serialVersionUID` — nearly every `Serializable` bean — therefore failed:
```
java.lang.IllegalArgumentException: non-null object for
private static final long
org.springframework.security.core.authority.SimpleGrantedAuthority.serialVersionUID
```
The modifier checks now run first so they short-circuit before `canAccess`.
This is a regression in the 8.x line from `9e60b8a4de`, which mechanically
swapped the non-throwing `isAccessible()` for `canAccess(o)`.
## One change beyond the two issues
Groovy compiles the variables captured by an anonymous class into
`ACC_PUBLIC | ACC_SYNTHETIC` `groovy.lang.Reference` fields. Once the field
loops could actually read them, they were emitted as duplicate keys with
empty-object values (`{"name":{},"age":{}}`) — the new tests caught exactly
that. Synthetic fields are compiler artifacts and never part of a bean's state,
so they are now skipped in all four marshallers.
## Files changed
- `grails-converters/.../marshaller/json/GroovyBeanMarshaller.java`
- `grails-converters/.../marshaller/json/GenericJavaBeanMarshaller.java`
- `grails-converters/.../marshaller/xml/GroovyBeanMarshaller.java`
- `grails-converters/.../marshaller/xml/GenericJavaBeanMarshaller.java`
## Tests
New fixtures under `org.grails.web.converters.beans`, deliberately in a
different package from the marshallers — in the same package the JVM access
check passes and neither bug reproduces. They cover a public interface with
both abstract and `default` read methods (mirroring `UserDetails`), Groovy and
Java anonymous / package-private / no-interface implementations, an anonymous
class with a declared public field, and a `Serializable` bean carrying `private
static final long serialVersionUID` alongside a public constant and a public
instance field.
Two new specs of 9 features each, driven through the public `new JSON(x)` /
`new XML(x)` API: `json.NonPublicClassMarshallingSpec` and
`xml.NonPublicClassMarshallingSpec`. Every non-public case first asserts
`!Modifier.isPublic(person.getClass().modifiers)`, so the tests fail loudly
rather than silently passing if a future compiler stops producing a non-public
class.
With the production change reverted, **18 of 18 fail** with the two reported
exceptions verbatim; with it, 18/18 pass. Also green locally: full
`:grails-converters:test`, `:grails-rest-transforms:test`,
`:grails-test-suite-web:test`, `:grails-test-suite-uber:test`,
`:grails-web-common:test` and `:grails-converters:codeStyle`.
No documentation change: this restores the documented behaviour of `as JSON`
/ `as XML` and adds no public API.
--
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]
