subclasses of Serialization

65 views
Skip to first unread message

Raquel Pau Fernández

unread,
Dec 21, 2015, 9:22:18 AM12/21/15
to Jenkins Developers
Hi,

I have sent a PR that removes dead code (https://github.com/jenkinsci/jenkins/pull/1957). Some of the suggested changes are to remove readResolve methods because the classes that contain those methods do not implement java.io.Serializable. XStream, as far as I know, uses the readResolve when it is dealing with the ObjectOutputStream and according the official Java documentation, just Serialiable and Externalizable elements can use readResolve. 

Then, my question is if these classes:

  • hudson.model.ListView
  • hudson.model.PaneStatusProperties
  • hudson.model.ParametersAction
  • hudson.model.Queue
  • hudson.security.csrf.DefaultCrumbIssuer
  • hudson.tasks.BatchFile
  • hudson.tasks.BuildTrigger
  • hudson.tasks.Shell
  • hudson.tools.BatchCommandInstaller
  • hudson.tools.CommandInstaller
  • hudson.util.RobustReflectionConverter
  • jenkins.model.ArtifactManagerConfiguration
  • jenkins.security.QueueItemAuthenticatorConfiguration
should be java.io.Serializable or the methods need to be removed?

Thank you in advance

Stephen Connolly

unread,
Dec 21, 2015, 9:58:55 AM12/21/15
to jenkin...@googlegroups.com


On Monday 21 December 2015, Raquel Pau Fernández <raqu...@gmail.com> wrote:
Hi,

I have sent a PR that removes dead code (https://github.com/jenkinsci/jenkins/pull/1957). Some of the suggested changes are to remove readResolve methods because the classes that contain those methods do not implement java.io.Serializable.


-1000

Those readResolve methods are critical for XStream
 
 XStream, as far as I know, uses the readResolve when it is dealing with the ObjectOutputStream and according the official Java documentation, just Serialiable and Externalizable elements can use readResolve. 


Then, my question is if these classes:

  • hudson.model.ListView
  • hudson.model.PaneStatusProperties
  • hudson.model.ParametersAction
  • hudson.model.Queue
  • hudson.security.csrf.DefaultCrumbIssuer
  • hudson.tasks.BatchFile
  • hudson.tasks.BuildTrigger
  • hudson.tasks.Shell
  • hudson.tools.BatchCommandInstaller
  • hudson.tools.CommandInstaller
  • hudson.util.RobustReflectionConverter
  • jenkins.model.ArtifactManagerConfiguration
  • jenkins.security.QueueItemAuthenticatorConfiguration
should be java.io.Serializable or the methods need to be removed?

Neither.

It might be beneficial to make them Serializable, but there is no requirement to do so. XStream.toXML doesn't need to take a Serializable and will use writeReplace methods if present... Vice versa for fromXML and readResolve

In general these classes may not be Serializable as they are not viewed safe for sending over remoting (which uses ObectOutputStream) so there are two different needs and hence 4 different scenarios, only one of which doesn't need readResolve
 

Thank you in advance


 

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/d670360c-93f8-4382-84f4-694039a02f8f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
Sent from my phone

Baptiste Mathus

unread,
Dec 21, 2015, 10:27:54 AM12/21/15
to jenkin...@googlegroups.com
@Stephen: then wouldn't at least a PR to add a comment to the readResolve() like https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/ListView.java#L112 be beneficial both for the common knowledge and for you not to have to repeat yourself every few weeks? :-)

Something short like:

/**
 */

(Note: I can prepare that PR if so, or I wouldn't propose it).

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CA%2BnPnMwt5Co3qBe55Mv1YbazWydjW7vqf5Ph4bs1g-UtmNubGQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.



--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

Raquel Pau Fernández

unread,
Dec 21, 2015, 11:38:22 AM12/21/15
to Jenkins Developers
Hi,

Thank you for your response. My answers below:

1) Just checking the source code of the xstream (https://github.com/x-stream/xstream/blob/8ea8038f96c5638315d026d5884603c9ce6cebd3/xstream/src/java/com/thoughtworks/xstream/core/util/SerializationMembers.java) I could verify what you are saying. In any place appear such information and it is not what Oracle says. In fact, it is an inconsistency that the system serializes objects that are not "Serializable".

2) "In general these classes may not be Serializable as they are not viewed safe for sending over remoting (which uses ObectOutputStream) so there are two different needs and hence 4 different scenarios, only one of which doesn't need readResolve". 

According the wikipedia definition of "Serialization", if you are translating object state into a format that can be stored, you are serializing (even if it these objects are not sent remotely). So may be, the good approach to deal with it is to check another interface rather that java.io.Serializable to send the object remotely.

Anyway, no problem, I can fix this behavior checking if xstream is in the classpath.

+1000 for your time ;)
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

Stephen Connolly

unread,
Dec 21, 2015, 4:34:53 PM12/21/15
to jenkin...@googlegroups.com
I would rather they all be Serializable so it really needs a case by case evaluation
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS6Hjo8o2OEv6H7Z1%3DrgYogFRWRnMMZsQNAdb9HBrJnVMA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

Raquel Pau Fernández

unread,
Dec 22, 2015, 7:58:55 AM12/22/15
to Jenkins Developers
@Stephen,

Thank you for your answer. Could you write a test to validate if a class that is Serializable provokes an undesired behavior?



El lunes, 21 de diciembre de 2015, 22:34:53 (UTC+1), Stephen Connolly escribió:
I would rather they all be Serializable so it really needs a case by case evaluation

On Monday 21 December 2015, Baptiste Mathus <m...@batmat.net> wrote:
@Stephen: then wouldn't at least a PR to add a comment to the readResolve() like https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/ListView.java#L112 be beneficial both for the common knowledge and for you not to have to repeat yourself every few weeks? :-)

Something short like:

/**
 */

(Note: I can prepare that PR if so, or I wouldn't propose it).
 

Thank you in advance


 
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
--
Sent from my phone

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.



--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

Stephen Connolly

unread,
Dec 22, 2015, 8:20:15 AM12/22/15
to jenkin...@googlegroups.com
No. This requires case by case analysis


On Tuesday 22 December 2015, Raquel Pau Fernández <raqu...@gmail.com> wrote:
@Stephen,

Thank you for your answer. Could you write a test to validate if a class that is Serializable provokes an undesired behavior?



El lunes, 21 de diciembre de 2015, 22:34:53 (UTC+1), Stephen Connolly escribió:
I would rather they all be Serializable so it really needs a case by case evaluation

On Monday 21 December 2015, Baptiste Mathus <m...@batmat.net> wrote:
@Stephen: then wouldn't at least a PR to add a comment to the readResolve() like https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/ListView.java#L112 be beneficial both for the common knowledge and for you not to have to repeat yourself every few weeks? :-)

Something short like:

/**
 */

(Note: I can prepare that PR if so, or I wouldn't propose it).
 

Thank you in advance


 
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
--
Sent from my phone

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.



--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.


--
Sent from my phone

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/29bc768d-b55b-482a-a0f6-479dd537e6cb%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Raquel Pau Fernández

unread,
Dec 22, 2015, 8:31:51 AM12/22/15
to Jenkins Developers
Sorry, 

but I would like to know/understand why creating a test is not useful to validate if these classes do not need to be Serializable? 

Thanks
 

Thank you in advance


 
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
--
Sent from my phone

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.



--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.


--
Sent from my phone

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

Jesse Glick

unread,
Dec 22, 2015, 10:16:16 AM12/22/15
to Jenkins Dev

It is a feature of XStream that it does not require any marker interface. Its semantics are intentionally aligned with, but not identical to, Java serialization (which Jenkins also uses, for different purposes). Workflow code also uses JBoss Marshalling, again similar but not identical to Java serialization. There is no mechanical way to determine which, if any, of these systems is/are used on a given class in Jenkins; you just have to understand the code.

Raquel Pau Fernández

unread,
Dec 22, 2015, 2:39:25 PM12/22/15
to Jenkins Developers
Thanks Jesse for your feedback :-)

I really appreciate your explanation. I will remove from my walkmod configuration the "removal option" of the serialization methods. Tomorrow, I will try to send again another PR to discuss for the rest of the changes.

Can you give me some clues about what packages and classes I would need to take a look? (or just do I need to find dependencies for XStream and JBoss Marshalling? ) Is there any documentation in somewhere? I would like to understand the code and "try" to create a test for it. 

However, what I mean is that even the java.io.Serialiable is not necessary for these libraries, if you are serializing objects, these should belong to this interface. Indeed, this is the purpose of these methods. So, what I would like to do is thinking about I can create a test to validate if making them Serializable provokes an error. Would it be helpful as well to you?

Thanks again for your support

Slide

unread,
Dec 23, 2015, 9:55:38 AM12/23/15
to Jenkins Developers

I disagree, it would be a pain to have to mark everything that could be serializable with the interface. It works quite well right now as is.


--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/fb2bdd92-a86f-44b9-951f-f99a19ae6194%40googlegroups.com.

Kanstantsin Shautsou

unread,
Dec 23, 2015, 6:41:20 PM12/23/15
to Jenkins Developers



On Tuesday, December 22, 2015 at 10:39:25 PM UTC+3, Raquel Pau Fernández wrote:
Thanks Jesse for your feedback :-)

I really appreciate your explanation. I will remove from my walkmod configuration the "removal option" of the serialization methods. Tomorrow, I will try to send again another PR to discuss for the rest of the changes.

Can you give me some clues about what packages and classes I would need to take a look? (or just do I need to find dependencies for XStream and JBoss Marshalling? ) Is there any documentation in somewhere? I would like to understand the code and "try" to create a test for it. 
Sorry, but nobody will be able to provide such info as code base is huge and there are many more critical real issues. 

However, what I mean is that even the java.io.Serialiable is not necessary for these libraries, if you are serializing objects, these should belong to this interface. Indeed, this is the purpose of these methods. So, what I would like to do is thinking about I can create a test to validate if making them Serializable provokes an error. Would it be helpful as well to you?
Serializable is java serialization, if object not using it, then where it said that it should be marked?. If you have real Serialization issues (i.e. like my issue that i referenced or like stephen in CBJOC), then you are welcome for fixing.
Reply all
Reply to author
Forward
0 new messages