koraktor / steam-condenser

A multi-language library for querying the Steam Community, Source, GoldSrc servers and Steam master servers
https://koraktor.de/steam-condenser
Other
356 stars 67 forks source link

java rconExec exception #239

Closed raydan closed 10 years ago

raydan commented 10 years ago

all these exception not 100% happen jdk 7, windows 7

fix: unknown
-----------------------------------------
java.nio.BufferUnderflowException
    at java.nio.Buffer.nextGetIndex(Buffer.java:498)
    at java.nio.HeapByteBuffer.getInt(HeapByteBuffer.java:355)
    at com.github.koraktor.steamcondenser.PacketBuffer.getInt(PacketBuffer.java:65)
    at com.github.koraktor.steamcondenser.servers.packets.rcon.RCONPacketFactory.getPacketFromData(RCONPacketFactory.java:36)
    at com.github.koraktor.steamcondenser.servers.sockets.RCONSocket.getReply(RCONSocket.java:123)
    at com.github.koraktor.steamcondenser.servers.SourceServer.rconExec(SourceServer.java:185)
-----------------------------------------

fix: unknown
-----------------------------------------
java.lang.IllegalArgumentException
        at java.nio.Buffer.position(Buffer.java:236)
        at com.github.koraktor.steamcondenser.PacketBuffer.getString(PacketBuffer.java:104)
        at com.github.koraktor.steamcondenser.servers.packets.rcon.RCONPacketFactory.getPacketFromData(RCONPacketFactory.java:38)
        at com.github.koraktor.steamcondenser.servers.sockets.RCONSocket.getReply(RCONSocket.java:121)
        at com.github.koraktor.steamcondenser.servers.SourceServer.rconExec(SourceServer.java:185)
-----------------------------------------

fix: check receivedBytes > 0
-----------------------------------------
java.lang.ArrayIndexOutOfBoundsException
    at java.lang.System.arraycopy(Native Method)
    at com.github.koraktor.steamcondenser.servers.sockets.RCONSocket.getReply(RCONSocket.java:117)
    at com.github.koraktor.steamcondenser.servers.SourceServer.rconExec(SourceServer.java:185)
-----------------------------------------
koraktor commented 10 years ago

This is with -Dfile.encoding=UTF-8, right?

raydan commented 10 years ago

yes,

ArrayIndexOutOfBoundsException look like it got empty packet,

koraktor commented 10 years ago

Do you have additional information on the RCON commands that cause this behavior?

raydan commented 10 years ago

my error show it happen in any command,

sometime it happen when you send a command, but the server still changing map or just changed map

koraktor commented 10 years ago

In your last mail about the status problem you said, that it was working correctly for -Dfile.encoding=UTF-8. So "any command" doesn't seem exact. At least some commands should work.

I'm assuming that this is the same server as in #227. Is this correct?

raydan commented 10 years ago

these exception should happen what ever -Dfile.encoding=UTF-8 on or off because i log all execption when i try to fix #227, that why i find these exception

it just happen when rconExec receiving empty packet in RCONSocket.java:117

example rcon command: my_time 1389257752 z_b_point 22 de_dust

it not the rcon command problem

temp fix


        byte[] packetData = new byte[packetSize];
        int receivedBytes;
        do {
            receivedBytes = this.receivePacket(remainingBytes);
            if(receivedBytes > 0) {
                System.arraycopy(this.buffer.array(), 0, packetData, packetSize - remainingBytes, receivedBytes);
                remainingBytes -= receivedBytes;
            }
        } while(remainingBytes > 0);

        if(packetData.length == 0) {
            throw new SteamCondenserException("Empty packet.");
        }
raydan commented 10 years ago

java.lang.IllegalArgumentException cause by -Dfile.encoding=UTF-8

this.byteBuffer.position(this.byteBuffer.position() + dataString.getBytes().length + 1);

dataString.getBytes() length in UTF8 format may longer

koraktor commented 10 years ago

I'm still unable to reproduce this issue, even with empty RCON responses.

Debugging PacketBuffer#getString() does not indicate a problem (especially at the specified line) as the ByteBuffer should operate on the UTF-8 encoded data and is therefore bigger too. The same is true for RCONSocket#getReply() where ArrayIndexOutOfBoundsException occurs for you.

All these errors you're experiencing seem to be caused by one encoding related bug, but I'm unable to reproduce this using just -Dfile.encoding=UTF-8 on OS X. I'll try to check this again on a Windows box.

raydan commented 10 years ago

i make a small test with/without -Dfile.encoding=UTF-8

http://pastebin.com/9ByEkNgv

result with -Dfile.encoding=UTF-8

test1
Data length: 18
stringEnd: 16
byteBuffer_1 position: 0
data bytes length: 16

test2
Data length: 18
stringEnd: 16
byteBuffer_2 position: 0
data bytes length: 28
java.lang.IllegalArgumentException
        at java.nio.Buffer.position(Buffer.java:236)
        at SrvTest.getString_2(SrvTest.java:91)
        at SrvTest.test2(SrvTest.java:70)
        at SrvTest.<init>(SrvTest.java:27)
        at SrvTest.main(SrvTest.java:97)

result without -Dfile.encoding=UTF-8

test1
Data length: 18
stringEnd: 12
byteBuffer_1 position: 0
data bytes length: 12

test2
Data length: 18
stringEnd: 12
byteBuffer_2 position: 0
data bytes length: 17

dataString.getBytes should use charset, or which charset?

koraktor commented 10 years ago

Ok, I understand now. And with your code snippet I can easily reproduce the problem. But I cannot reproduce it with Steam Condenser with one of your servers (or any other server for that matter).

Probably, this line in PacketBuffer has to be changed to

String dataString = new String(remainingBytes, Charset.forName("UTF-8"));

But I'm not entirely sure about the implications of this change.

I tested Steam Condenser with Java 6 and 7 on OS X and could not reproduce this error, so I'm hesitating to just change this right now. I'm still have to test it on a Windows box, though. What version(s) of Java are you using?

raydan commented 10 years ago

i am using windows 7, jdk7

IllegalArgumentException 90% from "status" command i think some player name cause that problem

koraktor commented 10 years ago

Can you provide the output of status through other means when Steam Condenser fails? For example using con_logfile? I'm still trying to reproduce this on my test server and if it's really caused by a player's name there's a good chance I can do this with bots. So having the problematic names would be a good start.

Or is there any chance that the server you gave me RCON access to will have more users playing? Currently it seems to be always empty.

raydan commented 10 years ago

my java program try to catch IllegalArgumentException then call phprcon run that affected command and make a log

but i already patch steam-condenser in my program, IllegalArgumentException doesn't happen anymore, so i don't know what player name can cause that problem

here is my patch code, and using -Dfile.encoding=UTF-8

if(stringEnd == -1) {
            return null;
        } else {
            dataString = dataString.substring(0, stringEnd);
            int len = 0;
            try {
                len = dataString.getBytes("US-ASCII").length;
            } catch(UnsupportedEncodingException e) {
                len = dataString.getBytes().length;
            }
            this.byteBuffer.position(this.byteBuffer.position() + len + 1);

            return dataString;
        }

that server i gave you just test, no player will join. now, i try to remove the patch, wait phprcon result

raydan commented 10 years ago

server.cfg save as system default (1252, iso-8859-1) hostname "ø �"

koraktor commented 10 years ago

Copying you're input here and saving it into a ISO-8859-1 encoded file seems to be impossible, iconv, vim and Sublime Text complain about conversion errors.

Can you upload your server.cfg in the right encoding?

koraktor commented 10 years ago

Thank you. I've finally been able to reproduce this error. I already found a simple fix for this that's not depending on encoding.

koraktor commented 10 years ago

The commit above "only" fixes the IllegalArgumentException in PacketBuffer, but I'm confident that the other errors are subsequent. Can you try to reproduce those other ones with the new code from 1.3-stable?

raydan commented 10 years ago

java.lang.ArrayIndexOutOfBoundsException POC use same code in #227 but change to

String str = server.rconExec("changelevel de_dust2");

            LogData(str);
            Thread.sleep(1000);
            String str2 = server.rconExec("status");

            LogData(str2);

change Thread.sleep(1000); to different value if the exception not happen

just test with 1.3-stable, ArrayIndexOutOfBoundsException still happen

koraktor commented 10 years ago

Ok, the latest patch b4b99878ccc44dd6835458db5ea0f46e8cedb91c should resolve this, too.

Please note that this situation will now result in a correct RCONNoAuthException and you will have to handle it, e.g. reauthenticate.

koraktor commented 10 years ago

Closing as this should be resolved now.