Skip to content

Conversation

@grundprinzip
Copy link
Contributor

What changes were proposed in this pull request?

Different systems will need different metadata that is passed as the user context during the request. To be able to handle the different systems seamlessly, make the UserContext extensible with google.protobuf.Any.

Why are the changes needed?

Extensibility.

Does this PR introduce any user-facing change?

No

How was this patch tested?

UT

@grundprinzip grundprinzip marked this pull request as ready for review October 24, 2022 09:26
@grundprinzip
Copy link
Contributor Author

// Spark Connect leverages the Any protobuf type that can be used to inject arbitrary other
// messages into this message. Extensions are stored as a `repeated` type to be able to
// handle multiple active extensions.
repeated google.protobuf.Any extensions = 999;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my self education:

Is Any type potentially a cause of security holes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No any is just used to define extension points in the proto so that at the point of writing the proto we don't need to define message that is embedded in the proto.

Interpreting the serialized class is up to the consumer and follows the same security model as any other serialized code so we have to be careful but it's not a new threat.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Makes sense.

Copy link
Contributor

@amaliujia amaliujia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Only left an open question.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in e64afb6 Oct 25, 2022
DeZepTup pushed a commit to DeZepTup/spark-custom that referenced this pull request Oct 31, 2022
Different systems will need different metadata that is passed as the user context during the request. To be able to handle the different systems seamlessly, make the `UserContext` extensible with `google.protobuf.Any`.

Extensibility.

No

UT

Closes apache#38374 from grundprinzip/SPARK-40899.

Authored-by: Martin Grund <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
### What changes were proposed in this pull request?

Different systems will need different metadata that is passed as the user context during the request. To be able to handle the different systems seamlessly, make the `UserContext` extensible with `google.protobuf.Any`.

### Why are the changes needed?
Extensibility.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
UT

Closes apache#38374 from grundprinzip/SPARK-40899.

Authored-by: Martin Grund <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants