Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a serialVersionUID to hudson.EnvVars to avoid deserialisation errors #3897

Merged
merged 1 commit into from
Feb 20, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions core/src/main/java/hudson/EnvVars.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
* @author Kohsuke Kawaguchi
*/
public class EnvVars extends TreeMap<String,String> {
private static final long serialVersionUID = 4320331661987259022L;
Copy link
Member

Choose a reason for hiding this comment

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

Note that despite I fully agree this is a good idea to use the value that's generated by default in the errors you've seen, FWIW IIRC the generated value is not defined in the spec, i.e. it could still fail on various JDKs implementations.

But anyway, we cannot do better than that, and at least this will make it fixed for all implementations in the future so 👍.

Also, if we go this path, should we then review all Serializable classes in Jenkins Core? Before this issue arises on other classes?

Looking https://github.com/jenkinsci/jenkins/search?q=Serializable&unscoped_q=Serializable, it seems we would ideally need to patch a lot of others isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"the generated value is not defined in the spec"
Interesting, I wasn't aware of that.

"should we then review all Serializable classes"
I guess yes, ideally. Though its probably even more than that list as in this case the class only implements Serializable indirectly.

private static Logger LOGGER = Logger.getLogger(EnvVars.class.getName());
/**
* If this {@link EnvVars} object represents the whole environment variable set,
Expand Down