Bastian / bstats-metrics

The different bStats Metrics classes
https://bStats.org
MIT License
98 stars 114 forks source link

java.lang.IllegalStateException: bStats Metrics class has not been relocated correctly #92

Closed Michel-0 closed 3 years ago

Michel-0 commented 3 years ago

Hi, i just got started with bStats, mostly everything seems fine to me until i ran into following exception:

10:52:25 [WARNING] Exception encountered when loading plugin: <...>
java.lang.IllegalStateException: bStats Metrics class has not been relocated correctly!
    at org.bstats.MetricsBase.checkRelocation(MetricsBase.java:220)
    at org.bstats.MetricsBase.<init>(MetricsBase.java:104)
    at org.bstats.bungeecord.Metrics.<init>(Metrics.java:61)
    at minecraft.spigot.community.michel_0.discord.Bot.onEnable(Bot.java:102)
    at net.md_5.bungee.api.plugin.PluginManager.enablePlugins(PluginManager.java:265)
    at net.md_5.bungee.BungeeCord.start(BungeeCord.java:287)
    at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:67)
    at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15)

I did not use Maven or anything. I did download the GIT repo here (master branch, not single-file). Then i used the "Import" feature of eclipse to import the subfolders "base" & "bungeecord". image Seems fine to me. However i'm completely confused by the method causing the above mentioned exception:

/**
 * Checks that the class was properly relocated.
 */
private void checkRelocation() {
    // You can use the property to disable the check in your test environment
    if (System.getProperty("bstats.relocatecheck") == null || !System.getProperty("bstats.relocatecheck").equals("false")) {
        // Maven's Relocate is clever and changes strings, too. So we have to use this little "trick" ... :D
        final String defaultPackage = new String( new byte[] {'o', 'r', 'g', '.', 'b', 's', 't', 'a', 't', 's'});
        final String examplePackage = new String(new byte[] {'y', 'o', 'u', 'r', '.', 'p', 'a', 'c', 'k', 'a', 'g', 'e'});
        // We want to make sure no one just copy & pastes the example and uses the wrong package names
        if (MetricsBase.class.getPackage().getName().startsWith(defaultPackage) || MetricsBase.class.getPackage().getName().startsWith(examplePackage)) {
            throw new IllegalStateException("bStats Metrics class has not been relocated correctly!");
        }
    }
}

Like what? I don't get it... org.bstats.MetricsBase is your class! Do you want me to relocate it? Why? It's yours, not mine! Also if i would relocate org.bstats.MetricsBase, i would also need to modify the import at org.bstats.bungeecord.Metrics.java, which is also your class... do you expect me to modify your library classes to be able to use it?

Am i missing something obvious or what's wrong here?

Also off-topic additional random question: Is there any security check other people won't use my Plugin ID? Seems like anyone could pass my Plugin ID to the Metrics constructor...? Doesn't sound safe to me.

Kind regards, Michel-0

Bastian commented 3 years ago

You are not supposed to copy the GitHub repo into your project. If you don't want to use a build tool like Gradle or Maven, just copy the Metrics.java file from the single-file branch into your project. Then you only have to update the package of this single file.

Also off-topic additional random question: Is there any security check other people won't use my Plugin ID? Seems like anyone could pass my Plugin ID to the Metrics constructor...? Doesn't sound safe to me.

There are some heuristics to try to detect malicious requests + manually banning of IPs of misbehaving servers. However, you are correct: It's not 100% failure-proof but there's no way to make this secure (since any form of "secret" could easily be obtained by decompiling your plugin).

Michel-0 commented 3 years ago

Hi, thanks for your fast reply. I see the point with the single-file Metrics.java... But i'm still not sure how i should interpret the sentence "Make sure to change the package from org.bstats to your own package." quote from here. Since the single file Metrics.java contains #checkRelocation as well, it would throw the same exception, won't it?

So even with this single-file Metrics.java you want me to put it into my package and modify the package name of your class? That's the thing i'm curious about... what's the point about that?

Bastian commented 3 years ago

Since the single file Metrics.java contains #checkRelocation as well, it would throw the same exception, won't it?

Yes. You must move the Metrics file into your own package, unique to your plugin. It's just simpler for the single file class since it's only a single line that you have to change.

what's the point about that?

Bukkit/Spigot (and afaik also Bungeecord and Velocity) has a shared class loader for all plugins. If multiple plugins use bStats with the same package, this could cause conflicts if they use different versions of bStats. In Sponge a relocation is technically not necessary but is still enforced for consistency reasons with the other Metrics classes.

Michel-0 commented 3 years ago

Ah yea ok i think i got your point. Moving it into my package basically makes it a standalone independent thing of my specific plugin. So it wouldn't matter whatever other plugins do with whatever maybe different version of bStats... Altough i don't really like to publish other peoples libraries with my package name, it makes some sense. I think i will put it into a new subpackage of yours like "org.bstats.michel_0.something" rather than into my package.

Thanks again for your fast and friendly replies, i think we consider this "issue" solved.