Repository navigation
fix: Default to GsonFactory and lazy-load default JsonFactory - #1257
Open
lahirumaramba wants to merge 1 commit into
Open
lahirumaramba wants to merge 1 commit into
lahirumaramba wants to merge 1 commit into
Conversation
Switch the default JsonFactory in ApiClientUtils from JacksonFactory to GsonFactory and initialize it lazily via a static holder class. Previously, ApiClientUtils referenced JacksonFactory directly even though firebase-admin only received google-http-client-jackson2 transitively from google-cloud-storage. Excluding Cloud Storage or Jackson2 caused a ClassNotFoundException when the JVM verified ApiClientUtils, even when callers explicitly passed a custom JsonFactory to FirebaseOptions. The upstream Gson regressions from 2021 (strict parsing, IOException on malformed JSON, and float serialization) are resolved in google-http-client-gson 2.x, which firebase-admin already depends on. Fixes #1231
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the default JsonFactory implementation from JacksonFactory to GsonFactory in ApiClientUtils. It introduces a lazy-loaded JsonFactoryInstanceHolder to hold the singleton GsonFactory instance and adds Javadoc documentation to the utility methods. Additionally, the test suite is updated to assert that GsonFactory is used by default, and new integration tests are added using a custom class loader to verify that the SDK functions correctly even when Jackson2 is absent from the classpath. There are no review comments, and I have no additional feedback to provide.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today, the SDK defaults to
JacksonFactoryfor JSON parsing even thoughfirebase-admindoes not declaregoogle-http-client-jackson2as a direct dependency (it only enters the classpath transitively viagoogle-cloud-storage). When an application excludesgoogle-cloud-storageorgoogle-http-client-jackson2, making an HTTP call triggers JVM bytecode verification ofApiClientUtilsand fails withNoClassDefFoundError, even when the caller explicitly passes a customJsonFactoryviaFirebaseOptions.This change switches the default
JsonFactorytoGsonFactory(whichfirebase-admin,google-api-client, andgoogle-auth-libraryalready depend on directly) and moves defaultJsonFactoryinitialization into a lazy holder class so customJsonFactoryconfigurations never load the default implementation. The upstreamGsonFactoryleniency issues that prevented this migration in 2021 (googleapis/google-api-java-client#1779) are resolved ingoogle-http-client-gson2.x, where strict parsing (readLeniency = false) is the default.Fixes #1231
Technical Details
ApiClientUtils): Previously,ApiClientUtils.getDefaultJsonFactory()returnedJacksonFactory.getDefaultInstance()directly as aJsonFactory, forcing the JVM bytecode verifier to eagerly loadJacksonFactorywheneverApiClientUtilswas loaded (for example, insideApiClientUtils.newAuthorizedRequestFactory(app)) to verify subtype assignability. WrappingGsonFactory.getDefaultInstance()inprivate static class JsonFactoryInstanceHolder(matchingTransportInstanceHolder) defers loading untilgetDefaultJsonFactory()is called.FirebaseOptions.getJsonFactory()exposes only the abstractJsonFactorytype, andpom.xmldependencies are unchanged. Malformed JSON inputs continue to throwIOException(MalformedJsonException), untyped JSON numbers parse toBigDecimal, and floats serialize identically (0.2f->"0.2"). Supplementary Unicode code points (such as emojis inTemplate.toJSON()) are serialized byGsonFactoryas literal UTF-8 rather than UTF-16 surrogate pairs, both of which are valid RFC 8259 JSON.ApiClientUtilsTest,FirebaseOptionsTest): AddedPackageBlockingClassLoadertests verifying that FCM requests and Remote ConfigTemplateserialization succeed whengoogle-http-client-jackson2andcom.fasterxml.jackson.*are absent from the classpath, and that passing a customJsonFactorydoes not load the defaultJsonFactoryimplementation class.