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

[FIXED JENKINS-51037] Add ConcurrentSkipListSet to whitelisted classes #3411

Closed
wants to merge 1 commit into from

Conversation

@jvz
Copy link
Member

jvz commented Apr 27, 2018

This adds ConcurrentSkipListSet to the list of default whitelisted classes for deserialization.

See JEP-200 and JENKINS-51037.

Proposed changelog entries

  • Internal: added java.util.concurrent.ConcurrentSkipListSet to whitelisted classes for serialization.

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@reviewbybees @dwnusbaum @daniel-beck

This adds ConcurrentSkipListSet to the list of default whitelisted
classes for deserialization.

See JEP-200.
@jglick
Copy link
Member

jglick commented Apr 27, 2018

Who is actually using a field of this type? Code reference? The JIRA issue gives no context. Generally we add whitelist entries only in response to observed failures.

I would also note that whoever is defining a persistent field of this type should stop doing so. Switch the field to some simpler type like Set; and, if the problem is via XStream define a readResolve to clean up old settings. (For Remoting usages, there is no diachronic consideration.) Whatever code is actually collecting entries concurrently can write out the result in a simple form when it is done.

@jvz
Copy link
Member Author

jvz commented Apr 27, 2018

It was in some wip code, but I can take a look into using readResolve instead.

@jvz
Copy link
Member Author

jvz commented Apr 27, 2018

Nevermind, this class isn't needed.

@jvz jvz closed this Apr 27, 2018
@jvz jvz deleted the jvz:add-skip-list-set branch Apr 27, 2018
@oleg-nenashev
Copy link
Member

oleg-nenashev commented Apr 27, 2018

For the record, I see no problem in adding it, just in case. But yes, we have different approaches on that with @jglick :D

@jvz
Copy link
Member Author

jvz commented Apr 27, 2018

I think it's better to be more explicit in your serialization anyways to avoid issues like this. I ended up using a proxy via readResolve/writeReplace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.