Skip to content

Remove Serializable from internal Type implementation classes.#3011

Merged
eamonnmcmanus merged 2 commits intogoogle:mainfrom
eamonnmcmanus:notserializable
Apr 10, 2026
Merged

Remove Serializable from internal Type implementation classes.#3011
eamonnmcmanus merged 2 commits intogoogle:mainfrom
eamonnmcmanus:notserializable

Conversation

@eamonnmcmanus
Copy link
Copy Markdown
Member

The nested classes ParameterizedTypeImpl, GenericArrayTypeImpl, and
WildcardTypeImpl in GsonTypes are implementations of the
corresponding types (without Impl) in java.lang.reflect. For some
reason, they have always implemented Serializable. They are even
documented that way, though the documentation is not published since
GsonTypes is an internal implementation class.

The implementations of these interfaces that are returned by
java.lang.reflect methods such as Method.getGenericReturnType() do
not implement Serializable so it seems gratuitous for Gson's
implementations to do so. Additionally, Serializable classes can
potentially participate in exploits. I think it is highly unlikely that
there is any practical exploit using these classes, but if we can avoid
even having to ask the question then we should.

The nested classes `ParameterizedTypeImpl`, `GenericArrayTypeImpl`, and
`WildcardTypeImpl` in `GsonTypes` are implementations of the
corresponding types (without `Impl`) in `java.lang.reflect`. For some
reason, they have always implemented `Serializable`. They are even
documented that way, though the documentation is not published since
`GsonTypes` is an internal implementation class.

The implementations of these interfaces that are returned by
`java.lang.reflect` methods such as `Method.getGenericReturnType()` do
*not* implement `Serializable` so it seems gratuitous for Gson's
implementations to do so. Additionally, `Serializable` classes can
potentially participate in exploits. I think it is highly unlikely that
there is any practical exploit using these classes, but if we can avoid
even having to ask the question then we should.
@eamonnmcmanus eamonnmcmanus requested a review from cpovirk April 10, 2026 18:02
Copy link
Copy Markdown
Member

@cpovirk cpovirk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent a short time but probably still more that was really justified to trace this:

That didn't really answer my question, but it's not like I expect serialization to turn out to be necessary. (I notice that the JDK implementation isn't Serializable, either, at least for ParameterizedType, which was the only one that I checked.)

@eamonnmcmanus
Copy link
Copy Markdown
Member Author

(I notice that the JDK implementation isn't Serializable, either, at least for ParameterizedType, which was the only one that I checked.)

Right, I mentioned that in the PR description, which is one reason why I feel pretty confident in making this change. (I also ran this change against all of Googe's internal tests and found no issues.)

@eamonnmcmanus eamonnmcmanus merged commit 0189b72 into google:main Apr 10, 2026
16 checks passed
@eamonnmcmanus eamonnmcmanus deleted the notserializable branch April 10, 2026 18:45
@cpovirk
Copy link
Copy Markdown
Member

cpovirk commented Apr 10, 2026

Oops, sorry, I must have clicked the link to jump straight to the diffs :)

return new WildcardTypeImpl(w.getUpperBounds(), w.getLowerBounds());

} else {
// type is either serializable as-is or unsupported
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this comment be removed as well (or adjusted, to mention that type is unsupported)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed! Feel free to send a PR. :-)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have created #3012; I hope the wording is ok. Please let me know if you want it changed, or want the comment to be removed completely.

mergify Bot added a commit to ArcadeData/arcadedb that referenced this pull request Apr 27, 2026
…ip ci]

Bumps [com.google.code.gson:gson](https://github.com/google/gson) from 2.13.2 to 2.14.0.
Release notes

*Sourced from [com.google.code.gson:gson's releases](https://github.com/google/gson/releases).*

> Gson 2.14.0
> -----------
>
> What's Changed
> --------------
>
> * Add type adapters for `java.time` classes by [`@​eamonnmcmanus`](https://github.com/eamonnmcmanus) in [google/gson#2948](https://redirect.github.com/google/gson/pull/2948)
>
>   When the `java.time` API is available, Gson automatically can read and write instances of classes like `Instant` and `Duration`. The format it uses essentially freezes the JSON representation that `ReflectiveTypeAdapterFactory` established by default, based on the private fields of `java.time` classes. That's not a great representation, but it is understandable. Changing it to anything else would break compatibility with systems that are expecting the current format.
>
>   With this change, Gson no longer tries to access private fields of these classes using reflection. So it is no longer necessary to run with `--add-opens` for these classes on recent JDKs.
> * Remove `com.google.gson.graph` by [`@​eamonnmcmanus`](https://github.com/eamonnmcmanus) in [google/gson#2990](https://redirect.github.com/google/gson/pull/2990).
>
>   This package was not part of any released artifact and depended on Gson internals in potentially problematic ways.
> * Validate that strings being parsed as integers consist of ASCII characters by [`@​eamonnmcmanus`](https://github.com/eamonnmcmanus) in [google/gson#2995](https://redirect.github.com/google/gson/pull/2995)
>
>   Previously, strings could contain non-ASCII Unicode digits and still be parsed as integers. That's inconsistent with how JSON numbers are treated.
> * Fix duplicate key detection when first value is null by [`@​andrewstellman`](https://github.com/andrewstellman) in [google/gson#3006](https://redirect.github.com/google/gson/pull/3006)
>
>   This could potentially break code that was relying on the incorrect behaviour. For example, this JSON string was previously accepted but will no longer be: `{"foo": null, "foo": bar}`.
> * Remove `Serializable` from internal `Type` implementation classes. by [`@​eamonnmcmanus`](https://github.com/eamonnmcmanus) in [google/gson#3011](https://redirect.github.com/google/gson/pull/3011)
>
>   The nested classes `ParameterizedTypeImpl`, `GenericArrayTypeImpl`, and `WildcardTypeImpl` in `GsonTypes` are implementations of the corresponding types (without `Impl`) in `java.lang.reflect`. For some reason, they were serializable, even though the `java.lang.reflect` implementations are not. Having unnecessarily serializable classes could *conceivably* have been a security problem if they were part of a larger exploit using serialization. (We do not consider this a likely scenario and do not suggest that you need to update Gson just to get this change.)
> * Add `LegacyProtoTypeAdapterFactory`. by [`@​eamonnmcmanus`](https://github.com/eamonnmcmanus) in [google/gson#3014](https://redirect.github.com/google/gson/pull/3014)
>
>   This is not part of any released artifact, but may be of use when trying to fix code that is currently accessing the internals of protobuf classes via reflection.
> * Make AppendableWriter do flush and close if delegation object supports by [`@​MukjepScarlet`](https://github.com/MukjepScarlet) in [google/gson#2925](https://redirect.github.com/google/gson/pull/2925)
>
> Other less visible changes
> --------------------------
>
> * Add default capacity to EnumTypeAdapter maps by [`@​MukjepScarlet`](https://github.com/MukjepScarlet) in [google/gson#2959](https://redirect.github.com/google/gson/pull/2959)
> * refactor: move derived adapters from Gson to TypeAdapters by [`@​MukjepScarlet`](https://github.com/MukjepScarlet) in [google/gson#2951](https://redirect.github.com/google/gson/pull/2951)
> * Optimize `new Gson()` by [`@​MukjepScarlet`](https://github.com/MukjepScarlet) in [google/gson#2864](https://redirect.github.com/google/gson/pull/2864)
>
> New Contributors
> ----------------
>
> * [`@​ThirdGoddess`](https://github.com/ThirdGoddess) made their first contribution in [google/gson#2944](https://redirect.github.com/google/gson/pull/2944)
> * [`@​lmj798`](https://github.com/lmj798) made their first contribution in [google/gson#2988](https://redirect.github.com/google/gson/pull/2988)
> * [`@​Eng-YasminKotb`](https://github.com/Eng-YasminKotb) made their first contribution in [google/gson#3005](https://redirect.github.com/google/gson/pull/3005)
> * [`@​andrewstellman`](https://github.com/andrewstellman) made their first contribution in [google/gson#3006](https://redirect.github.com/google/gson/pull/3006)
>
> **Full Changelog**: <google/gson@gson-parent-2.13.2...gson-parent-2.14.0>


Commits

* [`3ff35d6`](google/gson@3ff35d6) [maven-release-plugin] prepare release gson-parent-2.14.0
* [`a3024fd`](google/gson@a3024fd) Bump the maven group with 13 updates ([#3002](https://redirect.github.com/google/gson/issues/3002))
* [`5689ffe`](google/gson@5689ffe) Bump the github-actions group across 1 directory with 3 updates ([#3018](https://redirect.github.com/google/gson/issues/3018))
* [`48db33c`](google/gson@48db33c) Add `LegacyProtoTypeAdapterFactory`. ([#3014](https://redirect.github.com/google/gson/issues/3014))
* [`53d703e`](google/gson@53d703e) Update outdated comment regarding serializable types ([#3012](https://redirect.github.com/google/gson/issues/3012))
* [`0189b72`](google/gson@0189b72) Remove `Serializable` from internal `Type` implementation classes. ([#3011](https://redirect.github.com/google/gson/issues/3011))
* [`f4d371d`](google/gson@f4d371d) Fix duplicate key detection when first value is null ([#3006](https://redirect.github.com/google/gson/issues/3006))
* [`27d9ba1`](google/gson@27d9ba1) Fix typo in README (JPMS dependencies section) ([#3005](https://redirect.github.com/google/gson/issues/3005))
* [`1fa9b7a`](google/gson@1fa9b7a) Validate that strings being parsed as integers consist of ASCII characters (#...
* [`b7d5954`](google/gson@b7d5954) Add iterator fail-fast tests for LinkedTreeMap.clear() ([#2992](https://redirect.github.com/google/gson/issues/2992))
* Additional commits viewable in [compare view](google/gson@gson-parent-2.13.2...gson-parent-2.14.0)
  
[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=com.google.code.gson:gson&package-manager=maven&previous-version=2.13.2&new-version=2.14.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
Dependabot commands and options
  
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot show  ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants