You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/01/12 17:33:42 UTC

[GitHub] [activemq] jbonofre opened a new pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

jbonofre opened a new pull request #608:
URL: https://github.com/apache/activemq/pull/608


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] coheigea commented on a change in pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
coheigea commented on a change in pull request #608:
URL: https://github.com/apache/activemq/pull/608#discussion_r556573125



##########
File path: activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java
##########
@@ -369,7 +369,10 @@ public SubSelectorClassObjectInputStream(InputStream is) throws IOException {
 
         @Override
         protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException {
-            if (!(desc.getName().equals("java.lang.String") || desc.getName().startsWith("java.util."))) {
+            if (!(desc.getName().startsWith("java.lang.")
+                    || desc.getName().startsWith("com.thoughtworks.xstream")

Review comment:
       Is there a way to check if xstream is enabled, and if so to include this check, otherwise leave it out?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] jbonofre commented on a change in pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #608:
URL: https://github.com/apache/activemq/pull/608#discussion_r556599648



##########
File path: activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java
##########
@@ -369,7 +369,10 @@ public SubSelectorClassObjectInputStream(InputStream is) throws IOException {
 
         @Override
         protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException {
-            if (!(desc.getName().equals("java.lang.String") || desc.getName().startsWith("java.util."))) {
+            if (!(desc.getName().startsWith("java.lang.")
+                    || desc.getName().startsWith("com.thoughtworks.xstream")

Review comment:
       I think it's worth to keep it (just in case for http transport connector). Thoughts ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] jbonofre commented on pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #608:
URL: https://github.com/apache/activemq/pull/608#issuecomment-758819740


   @coheigea mind to take a look ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] jbonofre commented on a change in pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #608:
URL: https://github.com/apache/activemq/pull/608#discussion_r556578792



##########
File path: activemq-client/src/main/java/org/apache/activemq/util/ClassLoadingAwareObjectInputStream.java
##########
@@ -40,7 +40,7 @@
     private final ClassLoader inLoader;
 
     static {
-        serializablePackages = System.getProperty("org.apache.activemq.SERIALIZABLE_PACKAGES","org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");
+        serializablePackages = System.getProperty("java.lang,org.apache.activemq.SERIALIZABLE_PACKAGES","org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");

Review comment:
       Oh yes, my bad ;) I'm fixing.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] coheigea commented on a change in pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
coheigea commented on a change in pull request #608:
URL: https://github.com/apache/activemq/pull/608#discussion_r556570256



##########
File path: activemq-client/src/main/java/org/apache/activemq/util/ClassLoadingAwareObjectInputStream.java
##########
@@ -40,7 +40,7 @@
     private final ClassLoader inLoader;
 
     static {
-        serializablePackages = System.getProperty("org.apache.activemq.SERIALIZABLE_PACKAGES","org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");
+        serializablePackages = System.getProperty("java.lang,org.apache.activemq.SERIALIZABLE_PACKAGES","org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");

Review comment:
       There's a typo here, it should be: 
   
   serializablePackages = System.getProperty("org.apache.activemq.SERIALIZABLE_PACKAGES","java.lang,org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] coheigea commented on a change in pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
coheigea commented on a change in pull request #608:
URL: https://github.com/apache/activemq/pull/608#discussion_r556570256



##########
File path: activemq-client/src/main/java/org/apache/activemq/util/ClassLoadingAwareObjectInputStream.java
##########
@@ -40,7 +40,7 @@
     private final ClassLoader inLoader;
 
     static {
-        serializablePackages = System.getProperty("org.apache.activemq.SERIALIZABLE_PACKAGES","org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");
+        serializablePackages = System.getProperty("java.lang,org.apache.activemq.SERIALIZABLE_PACKAGES","org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");

Review comment:
       There's a typo here, it should be: "java.lang","




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] jbonofre merged pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
jbonofre merged pull request #608:
URL: https://github.com/apache/activemq/pull/608


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] jbonofre commented on a change in pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #608:
URL: https://github.com/apache/activemq/pull/608#discussion_r556579809



##########
File path: activemq-broker/src/main/java/org/apache/activemq/plugin/SubQueueSelectorCacheBroker.java
##########
@@ -369,7 +369,10 @@ public SubSelectorClassObjectInputStream(InputStream is) throws IOException {
 
         @Override
         protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException {
-            if (!(desc.getName().equals("java.lang.String") || desc.getName().startsWith("java.util."))) {
+            if (!(desc.getName().startsWith("java.lang.")
+                    || desc.getName().startsWith("com.thoughtworks.xstream")

Review comment:
       It could be enabled when using HTTP transport connector.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq] jbonofre commented on pull request #608: [AMQ-8097] Deal with deserialization with xstream unmarshal poison ack

Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #608:
URL: https://github.com/apache/activemq/pull/608#issuecomment-759518151


   @coheigea I updated the PR, if you mind to do a new pass. Thanks !


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org