HIVE-26539: Prevent unsafe deserialization in PartitionExpressionForM… #3605
Conversation
kgyrtkirk
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
| @@ -228,6 +230,27 @@ public void setConf(Configuration conf) { | |||
| public Configuration getConf() { | |||
| return configuration; | |||
| } | |||
|
|
|||
| @Override | |||
| public com.esotericsoftware.kryo.Registration getRegistration(Class type) { | |||
There was a problem hiding this comment.
Just to make sure my understanding is correct, is this method called before the class being instantiated? So, we can avoid some classes be instantiated.
There was a problem hiding this comment.
I think so, getRegistration is place for check whether an unregistered class is encountered when registrationRequired is set to true:
https://github.com/EsotericSoftware/kryo/blob/master/src/com/esotericsoftware/kryo/Kryo.java#L552-L556
However we cannot simply enable registrationRequired for UDF deserialization, as non-transient fields in the UDF are arbitrary, it's difficult to confine the inputs.
Thank you for the review and comments!
| try (Input inp = new Input(new ByteArrayInputStream(bytes))) { | ||
| return (T) kryo.readClassAndObject(inp); | ||
| } finally { | ||
| kryo.setExprNodeFirst(false); |
There was a problem hiding this comment.
Can we reset this field in releaseKryo()?
|
Kudos, SonarCloud Quality Gate passed!
|
|
@omalley Hi Owen, could you please also take a look at the changes? Thank you in advance! |
…etastore
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?
TestMetastoreExpr/TestSerializationUtilities