Skip to content

Conversation

@ryandens
Copy link
Member

@ryandens ryandens commented Sep 26, 2023

@ryandens ryandens changed the title Add compatibility with Java modules ✨ Add compatibility with Java modules Sep 26, 2023
@ryandens ryandens force-pushed the ryandens/java-module-compatibility branch from 1cb7643 to 150e0e4 Compare September 28, 2023 18:18
@ryandens ryandens marked this pull request as ready for review September 28, 2023 18:22
@ryandens ryandens requested a review from nahsra September 28, 2023 18:22
public static String encode(final String s) {
return Escape.html(s);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't leave a comment on it, but the createSafeObjectInputStream() from this type I assumed would be in the Java 8 version of this class for the JAR, so that we could use it. This seems like it will be only available in Java 11.

I see a couple solutions, but it seems like we should create a new type, SandboxingObjectInputStream that contains just that particular method, and leave this file as is. That way, everyone can access SandboxingObjectInputStream, and the ObjectInputFilter-related will only be in the 11 binary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I hadn't noticed that, i saw the class name on ObjectInputFilters and just assumed the whole class was a a java 8 thing. I'll fix with the new type

jacoco
`jvm-test-suite`
id("com.netflix.nebula.contacts") version "7.0.1"
id("com.netflix.nebula.source-jar") version "20.3.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Mmmhmm. I definitely understand this. Nice!

Copy link
Member Author

Choose a reason for hiding this comment

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

haha - just the netflix cocktail of plugins for creating a maven release that passes sonatype validation

@ryandens ryandens requested a review from nahsra September 28, 2023 19:46
* href="https://cheatsheetseries.owasp.org/cheatsheets/Deserialization_Cheat_Sheet.html">OWASP
* Cheat Sheet</a>.
*/
public final class SafeObjectInputStream {
Copy link
Contributor

Choose a reason for hiding this comment

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

This API now feels pretty clunky:

ObjectInputStream ois = SafeObjectInputStream.createSafeObjectInputStream(is);

I feel like it should be one of these:

ObjectInputStream ois = LimitingObjectInputStream.from(is);
ObjectInputStream ois = ObjectInputStreams.disallowDangerousTypes(is);

I tried to avoid Safe* as a prefix for everything because although it may be "safe", if the developer doesn't understand how it's making it safe, I think there's less chance of them using it.

See the # Type and method names section in CONTRIBUTING.md.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 I renamed to ObjectInputStreams to better align with ObjectInputFilters, but I want to limit the scope of this PR beyond what's necessary as we're already changing build system, adding multiple test suites, and introducing the multi-release JAR

@ryandens ryandens requested a review from nahsra September 28, 2023 20:58
@ryandens ryandens force-pushed the ryandens/java-module-compatibility branch from 11616e7 to 82333fe Compare September 28, 2023 21:56
@ryandens ryandens merged commit af9a1a3 into main Sep 28, 2023
@ryandens ryandens deleted the ryandens/java-module-compatibility branch September 28, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants