views:

123

answers:

4

I'm a real newbie to java, so please excuse me if this is a hopelessly straightforward problem.

I have the following from my java game server:

// Get input from the client
    DataInputStream in = new DataInputStream (server.getInputStream());
    PrintStream out = new PrintStream(server.getOutputStream());
    disconnect=false;

    while((line = in.readLine().trim()) != null && !line.equals(".") && !line.equals("") && !disconnect) {
        System.out.println("Received "+line);

      if(line.equals("h")){
          out.println("h"+EOF); // Client handshake
          System.out.println("Matched 1");

      }else if (line.equals("<policy-file-request/>")) {
          out.println("..."+EOF); // Policy file
          System.out.println(server.getInetAddress()+": Policy Request");
          disconnect=true;
          System.out.println("Matched 2");

      }else if(line.substring(0,3).equals("GET")||line.substring(0,4).equals("POST")){
          out.println("HTTP/1.0 200 OK\nServer: VirtuaRoom v0.9\nContent-Type: text/html\n\n..."); // HTML status page
          disconnect=true;
          System.out.println("Matched 3");


      } else {
          System.out.println(server.getInetAddress()+": Unknown command, client disconnected.");
          disconnect=true;
          System.out.println("Matched else");

      }

    }
    server.close();

First of all, the client sends an "h" packet, and expects the same back (handshake). However, I want it to disconnect the client when an unrecognised packet is received. For some reason, it responds fine to the handshake and HTML status request, but the else clause is never executed when there's an unknown packet.

Thanks

+1  A: 
  1. You need to check for null before you trim it. The result of trim() can never be null.

  2. You should check disconnect first, before the readLine(), otherwise you are always doing one readLine() too many.

  3. If you are never getting to your 'else' it means one of the other conditions is always true.

EJP
@downvoters, thanks, -2. Why? Kindly point out the error in the above.
EJP
A: 

It would seem highly unlikely that the else is not executing. Are you sure your loop does not exit on such packets and hence your conditions do not even run? Does your System.out.println("Received "+line); print anything for the packet that seems to be missing the else statement?

filip-fku
+1  A: 

There are a number of problems with your code

  • in.readLine().trim() readLine do returns null and calling null.trim() will result in ... NullPointerException
  • Is there a reason to append EOF to every response you send.
  • calling substring without making sure it has at least that much elements will throw StringIndexOutOfBoundsException if it is shorter.

Are you testing with "P" for example?

Anton
+4  A: 

From the information added in your comments it seems that what will be happening is the client is sending a single character (e.g. 'n'). The line

line.substring(0,3).equals("GET")||line.substring(0,4).equals("POST"))

will be executed, but since line is only a single character line.substring(0,3) will throw a StringIndexOutOfBoundsException. Either this is causing your program to fail and you haven't mentioned that. Or you have some exception handling going on in another part of your code that you haven't shown and this is either supressing the error or printing a log line or something and again you haven't mentioned this (or noticed it).

Try replacing substring().equals with startsWith

DaveJohnston
This did the trick, thanks so much!I was in fact suppressing StringIndexOutOfBoundsException, had no idea that would affect it, thanks again!
Alex