CommandHelper

CommandHelper

46.5k Downloads

Player IP alteration that conflicts with other plugins

LadyCailin opened this issue ยท 8 comments

commented

CMDHELPER-2653 - Reported by kajar9 on 2013-04-21 05:42:09 UTC

Regular IP when CommandHelper isn't loaded : /124.184.xx.xxx
IP reported when CommandHelper is loaded : CPE-124-184-xx-xxx.lns14.cht.bigpond.net.au/124.184.xx.xxx
Sometimes it reports it like double instead with text : 124.84.xx.xxx/124.184.xx.xxx

This is directly related to CommandHelper, because if I start the server without CommandHelper it is normal.

Currently it gives me problems with plugins that store or use player IP. Example BanManager, Statistics, Essentials and so on.
How to disable it or how to fix it. It is a useless addition for me and shouldn't disrupt the workings of other plugins

THANK YOU!
Need ANSWER ASAP!

This happens with default configuration of CommandHelper. Can't be fixed by removing all config files and scripts. Or clearing them, excluding the option that it regenerates the bug via files. It must come from within the plugin itself.

FIX IT! NAOW!!!!

commented

Comment by LadyCailin on 2013-04-23 12:45:50 UTC

So you want to tell me that 4-5 (relatively popular) plugins are using a bad call for getting the players IP then.
And your plugin uses the correct way?!? You are breaking multiple plugins functionality (Essentials ex.) and yet you claim that this bug directly caused by your "correct" way of getting the IP is ok to break majority of plugins and claiming it to be an invalid claim?

Yes, that is what I'm saying. The code I posted clearly demonstrates the problem, and the solution. This is known behavior in the Java API, and if other plugin authors aren't aware of this, that is not MY BUG. I looked in BanManager's source, to find this example of awful programming: https://github.com/confuser/Ban-Management/blob/master/src/me/confuserr/banmanager/BanManager.java#L254

javareturn ip.toString().replace("/", "");

This example of terrible programming can be replaced with ip.getHostAddress() instead of using ip.toString(), which is the correct way of doing this. (I can't believe a ban plugin doesn't know how to do this correctly.) There is no workaround that I can provide, without removing functionality, so there is literally nothing for me to do here.

Also, I don't think you understand how this works, this is a volunteer operation, and your attitude is awful, even if this were my bug, you're being extremely rude.

commented

Comment by kajar9 on 2013-04-23 14:17:36 UTC

I am sorry if I am coming forward as rude, it wasn't in any way my goal. I let my frustration get over me.
I will try to contact other plugin developers (which seem to break) to fix the issue. I have posted noticed to plugin developers whose plugins are causing me that issue.

I deeply respect the work you do and am sorry for any misunderstanding!

commented

Comment by LadyCailin on 2013-04-23 15:41:59 UTC

That's ok.

You can show them the java code I posted, and in the case of BanManager specifically that particular line of code. Their intention is to get the raw IP, which is available with getHostAddress, in the case of BanManager, they are parsing the string returned by toString(), which will change once ANY code calls getHostName(). In addition to being the correct way to do it, it will actually simplify their code. If any of the developers don't know how to fix this, let me know, and I'll seek them out and work with them to help them fix it.

commented

Comment by LadyCailin on 2013-04-22 22:06:49 UTC

This is not a bug with CH. This is a bug with the other plugins. There is a difference between using InetAddress.toString() and InetAddress.getHostAddress(). The plugins are apparently using toString() to get the IP address, which is modified when you call InetAddress.getHostName(). Here's sample java code:

InetAddress i2 = InetAddress.getByName("173.194.37.72");
System.out.println("i2.toString(): " + i2.toString());
System.out.println("i2.getHostAddress(): " + i2.getHostAddress());
System.out.println("i2.getHostName(): " + i2.getHostName());
System.out.println("i2.toString(): " + i2.toString());
System.out.println("i2.getHostAddress(): " + i2.getHostAddress());

The output of this code is:

i2.toString(): /173.194.37.72
i2.getHostAddress(): 173.194.37.72
i2.getHostName(): atl14s08-in-f8.1e100.net
i2.toString(): atl14s08-in-f8.1e100.net/173.194.37.72
i2.getHostAddress(): 173.194.37.72

As you can see, the first call to toString returns "/173.194.37.72" yet after calling getHostName(), it returns the longer string. getHostAddress() returns the ip the same each time however. If the other plugin was storing the IP from the toString version, that is their fault, not mine.


FIX IT! NAOW!!!!

Sarcasm doesn't translate over text very well, so don't try to be funny like that, you just come off as rude.

commented

Comment by kajar9 on 2013-04-23 10:13:23 UTC

So you want to tell me that 4-5 (relatively popular) plugins are using a bad call for getting the players IP then.
And your plugin uses the correct way?!? You are breaking multiple plugins functionality (Essentials ex.) and yet you claim that this bug directly caused by your "correct" way of getting the IP is ok to break majority of plugins and claiming it to be an invalid claim?

These plugins have worked fine until I added CommandHelper.
Try having only Essentials and CommandHelper and the bug comes apparent.
Try any other plugin that had that bug with CommandHelper with eachother or any other plugin, it wont bug out.

Even if you claim you are using the "correct" way of using it. It does break other plugins so that is YOUR BUG.
If your "correct" way wouldn't interfere with any other plugins... then fine. But it does... So you should look into it and fix it. At least add some kind of workaround because asking 1 developer is much better for users than having to persuade 10+ or so to change instead.

commented

Comment by KHobbits on 2013-04-23 15:45:19 UTC

Just throwing a comment in here.

Essentials uses toString when displaying in /whois and /seen, and uses getHostAddress for banning, and IP comparison, thus there is no bug in Essentials.

commented

Comment by kajar9 on 2013-05-17 15:39:08 UTC

Could you review the code of Statistics plugin and see what the developer does wrong. He tried fixing it twice, in he's mind, but still without any success. It still logs the full IP from the user to the database creating errors of numerous sorts.

Thank you!

https://github.com/BukkitStatistics/Plugin/

commented

Comment by LadyCailin on 2013-05-17 16:25:12 UTC

BukkitStatistics/Plugin@d89eda7#commitcomment-3236489

They are still not explicitly calling the getHostName() method. The default toString() (which is what will probably eventually get called) is going to return the weird formatted IP.