-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Remove the default serializable packages and deprecated the property to introduce org.apache.avro.SERIALIZABLE_CLASSES instead #3376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java
Show resolved
Hide resolved
78ca676 to
26294d9
Compare
26294d9 to
5269613
Compare
|
@nandorKollar @martin-g @Fokko I updated the PR. Can you please do a new pass ? |
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java
Fixed
Show fixed
Hide fixed
nandorKollar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM! Thanks for fixing this!
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java
Outdated
Show resolved
Hide resolved
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead
5269613 to
e9bfef8
Compare
|
@martin-g @Fokko @nandorKollar I'm volunteer to do new Avro releases (1.11.5 and 1.12.1) including this. Thoughts ? |
| throw new SecurityException("Forbidden " + clazz | ||
| + "! This class is not trusted to be included in Avro schema using java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property with the packages you trust."); | ||
|
|
||
| for (String trustedPackage : getTrustedPackages()) { |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
SpecificDatumReader.getTrustedPackages
I'll wait a couple of more days for any objection, then I'll merge this PR. Would be great if you could help with the release! |
Fokko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for waiting @nandorKollar, my mailbox is overflowing. This looks good to me, thanks @jbonofre for working on this 👍
|
@jbonofre I'm seeing some errors on main: https://github.com/apache/avro/actions/runs/15121164134/job/42503656318 |
|
Ooops, looks like setting |
|
@nandorKollar @Fokko i was a out to fix the IT as we are now much more restrictive than before. Thanks @nandorKollar for fixing that in the other PR ! |
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead (apache#3376) (cherry picked from commit 90a937f)
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead (apache#3376) (cherry picked from commit 90a937f)
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead (apache#3376)
As discussed within the Parquet community, this PR is more "restrictive":
org.apache.avro.SERIALIZABLE_PACKAGESpropertySERIALIZABLE_PACKAGES, users should useorg.apache.avro.SERIALIZABLE_CLASSESThis is a much more "strict" security enforcement.