You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/09/01 09:12:55 UTC

[GitHub] [spark] fsamuel-bs opened a new pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

fsamuel-bs opened a new pull request #33879:
URL: https://github.com/apache/spark/pull/33879


   ## Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain)
   https://issues.apache.org/jira/browse/SPARK-36627
   
   ## What changes were proposed in this pull request?
   See issue above for issue description.
   
   ### Why are the changes needed?
   Spark deserialization fails with no recourse for the user.
   
   ### Does this PR introduce any user-facing change?
   No.
   
   ### How was this patch tested?
   Unit tests.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952983648


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144660/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-909427123


   Can one of the admins verify this patch?


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953855250


   Jenkins test this please


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953017357


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49131/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953982489


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49188/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953934513


   Kubernetes integration test starting
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49188/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-909427123


   Can one of the admins verify this patch?


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen closed pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen closed pull request #33879:
URL: https://github.com/apache/spark/pull/33879


   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953131690


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144662/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953078879


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49131/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952983551


   **[Test build #144660 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144660/testReport)** for PR 33879 at commit [`76f4fa6`](https://github.com/apache/spark/commit/76f4fa64de2e680ef6c2d466ae722ac10578f33e).
    * This patch **fails Java style tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953890345


   **[Test build #144719 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144719/testReport)** for PR 33879 at commit [`e6b0bf6`](https://github.com/apache/spark/commit/e6b0bf6cf537d77f4c413959c3c13d17db3a0c4f).


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952970787


   **[Test build #144660 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144660/testReport)** for PR 33879 at commit [`76f4fa6`](https://github.com/apache/spark/commit/76f4fa64de2e680ef6c2d466ae722ac10578f33e).


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952974654


   **[Test build #144662 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144662/testReport)** for PR 33879 at commit [`76f4fa6`](https://github.com/apache/spark/commit/76f4fa64de2e680ef6c2d466ae722ac10578f33e).


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-954292218


   Merged to master


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-927205189


   Out of curiosity, where do proxy classes typically come up?


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953131690


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144662/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953994728


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49188/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-954040788


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144719/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] fsamuel-bs commented on a change in pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
fsamuel-bs commented on a change in pull request #33879:
URL: https://github.com/apache/spark/pull/33879#discussion_r737523967



##########
File path: core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala
##########
@@ -28,8 +28,10 @@ import org.apache.spark.internal.config._
 import org.apache.spark.util.{ByteBufferInputStream, ByteBufferOutputStream, Utils}
 
 private[spark] class JavaSerializationStream(
-    out: OutputStream, counterReset: Int, extraDebugInfo: Boolean)
-  extends SerializationStream {
+    out: OutputStream,

Review comment:
       yep, all was scalafmt




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] fsamuel-bs commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
fsamuel-bs commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-924114490


   @EugenCepoi @squito could any of you take a look at this PR? Pinging you both because you seem to have worked together on https://issues.apache.org/jira/browse/SPARK-8730, https://github.com/apache/spark/pull/7122, which is similar to this one.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] fsamuel-bs commented on a change in pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
fsamuel-bs commented on a change in pull request #33879:
URL: https://github.com/apache/spark/pull/33879#discussion_r699528902



##########
File path: core/src/test/scala/org/apache/spark/serializer/JavaSerializerSuite.scala
##########
@@ -31,11 +31,31 @@ class JavaSerializerSuite extends SparkFunSuite {
   test("Deserialize object containing a primitive Class as attribute") {
     val serializer = new JavaSerializer(new SparkConf())
     val instance = serializer.newInstance()
-    val obj = instance.deserialize[ContainsPrimitiveClass](instance.serialize(
-      new ContainsPrimitiveClass()))
+    val obj = instance.deserialize[ContainsPrimitiveClass](
+      instance.serialize(new ContainsPrimitiveClass()))
     // enforce class cast
     obj.getClass
   }
+
+  test("SPARK-36627: Deserialize object containing a proxy Class as attribute") {
+    var classesLoaded = Set[String]()
+    val outer = Thread.currentThread.getContextClassLoader
+    val inner = new ClassLoader() {
+      override def loadClass(name: String): Class[_] = {
+        classesLoaded = classesLoaded + name
+        outer.loadClass(name)
+      }
+    }
+    Thread.currentThread.setContextClassLoader(inner)
+
+    val serializer = new JavaSerializer(new SparkConf())
+    val instance = serializer.newInstance()
+    val obj =
+      instance.deserialize[ContainsProxyClass](instance.serialize(new ContainsProxyClass()))
+    // enforce class cast
+    obj.getClass
+    assert(classesLoaded.exists(klass => klass.contains("MyInterface")))

Review comment:
       This is a **very** roundabout way of testing this, but I couldn't think of another way of testing it - the way we hit this issue was running spark in YARN, and I don't think we can trivially reproduce it with spark local.
   
   In order to repro exactly what happened in YARN we'd need to make `sun.misc.VM.latestUserDefinedLoader()` not be able to resolve `MyInterface`, but I couldn't figure out how to do it.

##########
File path: core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala
##########
@@ -71,6 +71,13 @@ private[spark] class JavaDeserializationStream(in: InputStream, loader: ClassLoa
         case e: ClassNotFoundException =>
           JavaDeserializationStream.primitiveMappings.getOrElse(desc.getName, throw e)
       }
+
+    override def resolveProxyClass(ifaces: Array[String]): Class[_] = {
+        // scalastyle:off classforname
+        val resolved = ifaces.map(iface => Class.forName(iface, false, loader))

Review comment:
       We definitely want better exceptions if this fails, but not sure what exactly we'd want here.




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] fsamuel-bs commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
fsamuel-bs commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-949460278


   Sorry for the late response - proxy classes come up when we're building http clients. We usually build them from interfaces (annotated with JaxRs annotations or custom ones) and reference it by its interface. See [feign](https://github.com/OpenFeign/feign/blob/aa96dda408159cd0c8b0222d6b2f3573932b7a12/core/src/main/java/feign/ReflectiveFeign.java#L65-L66) for an example.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952970787


   **[Test build #144660 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144660/testReport)** for PR 33879 at commit [`76f4fa6`](https://github.com/apache/spark/commit/76f4fa64de2e680ef6c2d466ae722ac10578f33e).


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on a change in pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #33879:
URL: https://github.com/apache/spark/pull/33879#discussion_r737507186



##########
File path: core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala
##########
@@ -28,8 +28,10 @@ import org.apache.spark.internal.config._
 import org.apache.spark.util.{ByteBufferInputStream, ByteBufferOutputStream, Utils}
 
 private[spark] class JavaSerializationStream(
-    out: OutputStream, counterReset: Int, extraDebugInfo: Boolean)
-  extends SerializationStream {
+    out: OutputStream,

Review comment:
       I think this is fine, if this is what scalafmt does to this file - otherwise I'd say leave it




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952983648


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144660/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] fsamuel-bs commented on a change in pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
fsamuel-bs commented on a change in pull request #33879:
URL: https://github.com/apache/spark/pull/33879#discussion_r737499002



##########
File path: core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala
##########
@@ -71,6 +71,13 @@ private[spark] class JavaDeserializationStream(in: InputStream, loader: ClassLoa
         case e: ClassNotFoundException =>
           JavaDeserializationStream.primitiveMappings.getOrElse(desc.getName, throw e)
       }
+
+    override def resolveProxyClass(ifaces: Array[String]): Class[_] = {
+        // scalastyle:off classforname
+        val resolved = ifaces.map(iface => Class.forName(iface, false, loader))

Review comment:
       `Utils#classForName` uses either `getContextOrSparkClassLoader` or `thread.currentThread().getContextClassLoader` which might not be the ones we want depending on the context we're deserializing -- I'd rather use the same classloader we're using above (when deserializing non-proxy java objects).




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953057658


   Kubernetes integration test status failure
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49131/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-954033769


   **[Test build #144719 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144719/testReport)** for PR 33879 at commit [`e6b0bf6`](https://github.com/apache/spark/commit/e6b0bf6cf537d77f4c413959c3c13d17db3a0c4f).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-954040788


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144719/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953890345


   **[Test build #144719 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144719/testReport)** for PR 33879 at commit [`e6b0bf6`](https://github.com/apache/spark/commit/e6b0bf6cf537d77f4c413959c3c13d17db3a0c4f).


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-909427123


   Can one of the admins verify this patch?


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on a change in pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #33879:
URL: https://github.com/apache/spark/pull/33879#discussion_r734551484



##########
File path: core/src/test/java/org/apache/spark/serializer/ProxySerializerTest.java
##########
@@ -0,0 +1,33 @@
+package org.apache.spark.serializer;

Review comment:
       Copyright header

##########
File path: core/src/test/java/org/apache/spark/serializer/ProxySerializerTest.java
##########
@@ -0,0 +1,33 @@
+package org.apache.spark.serializer;
+
+import java.io.Serializable;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+
+class ContainsProxyClass implements Serializable {
+    final MyInterface proxy = (MyInterface) Proxy.newProxyInstance(

Review comment:
       Likewise 2-space

##########
File path: core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala
##########
@@ -71,6 +71,13 @@ private[spark] class JavaDeserializationStream(in: InputStream, loader: ClassLoa
         case e: ClassNotFoundException =>
           JavaDeserializationStream.primitiveMappings.getOrElse(desc.getName, throw e)
       }
+
+    override def resolveProxyClass(ifaces: Array[String]): Class[_] = {
+        // scalastyle:off classforname
+        val resolved = ifaces.map(iface => Class.forName(iface, false, loader))

Review comment:
       2-space indent, and maybe you can use the Utils method in Spark for loading the class?




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953078879


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49131/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953994728


   
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49188/
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] fsamuel-bs commented on a change in pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
fsamuel-bs commented on a change in pull request #33879:
URL: https://github.com/apache/spark/pull/33879#discussion_r737500282



##########
File path: core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala
##########
@@ -141,20 +156,23 @@ class JavaSerializer(conf: SparkConf) extends Serializer with Externalizable {
   private var counterReset = conf.get(SERIALIZER_OBJECT_STREAM_RESET)
   private var extraDebugInfo = conf.get(SERIALIZER_EXTRA_DEBUG_INFO)
 
-  protected def this() = this(new SparkConf())  // For deserialization only
+  protected def this() = this(new SparkConf()) // For deserialization only
 
   override def newInstance(): SerializerInstance = {
     val classLoader = defaultClassLoader.getOrElse(Thread.currentThread.getContextClassLoader)
     new JavaSerializerInstance(counterReset, extraDebugInfo, classLoader)
   }
 
-  override def writeExternal(out: ObjectOutput): Unit = Utils.tryOrIOException {
-    out.writeInt(counterReset)
-    out.writeBoolean(extraDebugInfo)
-  }
+  override def writeExternal(out: ObjectOutput): Unit =

Review comment:
       Ran `./dev/scalafmt`, which modified a few more lines.




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952969658


   Jenkins test this please


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-953118271


   **[Test build #144662 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144662/testReport)** for PR 33879 at commit [`76f4fa6`](https://github.com/apache/spark/commit/76f4fa64de2e680ef6c2d466ae722ac10578f33e).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #33879: [SPARK-36627][CORE] Fix java deserialization of proxy classes

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33879:
URL: https://github.com/apache/spark/pull/33879#issuecomment-952974654


   **[Test build #144662 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144662/testReport)** for PR 33879 at commit [`76f4fa6`](https://github.com/apache/spark/commit/76f4fa64de2e680ef6c2d466ae722ac10578f33e).


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org