Skip to content

Fix NPE deserializing ServiceAccountCredentials#132

Merged
lesv merged 3 commits intogoogleapis:masterfrom
igorbernstein2:fix-deserialization
Sep 27, 2017
Merged

Fix NPE deserializing ServiceAccountCredentials#132
lesv merged 3 commits intogoogleapis:masterfrom
igorbernstein2:fix-deserialization

Conversation

@igorbernstein2
Copy link
Copy Markdown
Contributor

Deserialization of ServiceAccoutnCredentials broke in 0.7.0 via this commit:
432ee7e#diff-e186a7a41155398a3502025e16ac1508L479

It removed the java deserialization hook readObject to properly deserialize the transient transportFactory field.

This PR reverts that change and adds a test

@igorbernstein2
Copy link
Copy Markdown
Contributor Author

@garrettjonesgoogle @lesv @anthmgoogle: can someone review this? This bug breaks authentication via service accounts for dataflow io connectors. Please see b/65304954 for details

@garrettjonesgoogle
Copy link
Copy Markdown
Contributor

@vchudnov-g could you comment on why the readObject method was removed in your commit?

@vchudnov-g
Copy link
Copy Markdown
Contributor

As per my comments, the method seemed to be unused.

If this change bringing readObject back fixes things, I'm fine with it.

I'm still curious, though, where/how does readObject get invoked?

@garrettjonesgoogle
Copy link
Copy Markdown
Contributor

@vchudnov-g
Copy link
Copy Markdown
Contributor

OK, thanks for the pointer.

The fix is fine by me. Maybe include a comment saying the method is needed for the Serializable interface?

I get that this is the Way Things Are Done, but these types of magic invocations seem really counterintuitive and lend themselves to issues like these.....

At any rate, my apologies for breaking things.

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.

4 participants