stage2/ml9: Fix crash with SinytraConnector #25
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The "Sinytra Connector" mod uses a custom `SecureJar` implementation, which prior to this commit resulted in us doing an out of bounds memory read (returning what looks like a String, resulting in a class cast exception) because we were using the same unsafe getter that we were using for ML's `Jar` implementation. This commit fixes the issue by using a separate getter for each implementation class. This change alone is sufficient to fix the issue because we'll then get an explicit `NoSuchFieldException`, which we catch. However, to avoid unnessecary log spam, this commit additionally special cases the dummy class to silently return `null`. It also changes our `getVersion` method to still try the fallback paths (which do work!) instead of returning early on `null` since that is now a legitimate case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The "Sinytra Connector" mod uses a custom
SecureJarimplementation, which prior to this commit resulted in us doing an out of bounds memory read (returning what looks like a String, resulting in a class cast exception) because we were using the same unsafe getter that we were using for ML'sJarimplementation.This commit fixes the issue by using a separate getter for each implementation class.
This change alone is sufficient to fix the issue because we'll then get an explicit
NoSuchFieldException, which we catch. However, to avoid unnessecary log spam, this commit additionally special cases the dummy class to silently returnnull. It also changes ourgetVersionmethod to still try the fallback paths (which do work!) instead of returning early onnullsince that is now a legitimate case.Side note: While this is sufficient to get Essential working with Connector, we do still run into problems if fabric-language-kotlin is installed via it without KotlinForForge being installed. This is because Connector loads FLK into the GAME layer via FML's
IDependencyLocatormechanism (unless KFF is installed, in which case it skips loading it) while we load our Kotlin into the PLUGIN layer directly via ModLauncher. Fixing this is non-trivial and since Connector itself also misbehaves if a newer FLK is loaded while an older KFF is already present, I'll be ignoring that issue for the time being.