Comparing strings not working, and empty list not empty

Question

I am working on a little Server - Client program. The code I have here runs in a different program as well and works fine, but for some reason in this program does not.

So when the user wants to login they type their Username and Password this message is then sent as "loginreq user:pass" to the server socket.

Although the Client clearly isn't the problem (I think) I will provide the code for you:

private static void loginreq(){
    String User = null;
    String Pass = null;
    try {
        User = bufferRead.readLine();
        WriteOut(ANSI_RESET+"Password: " + ANSI_GREEN);
        Pass = bufferRead.readLine();
        WriteOut(ANSI_RESET);
    } catch (IOException e1) {
        e1.printStackTrace();
    }
    int serverPort = 6880;
      String ip = Server;
      String data = "loginreq " + User + ":" + Pass;
    try{ 
        Socket s = new Socket(ip, serverPort); 
        DataInputStream input = new DataInputStream( s.getInputStream()); 
      DataOutputStream output = new DataOutputStream( s.getOutputStream());
      if(s.isConnected()){
          //Step 1 send length
          System.out.println("Length"+ data.length());
          output.writeInt(data.length());
          //Step 2 send length
          System.out.println("Writing.......");
          output.writeBytes(data); // UTF is a string encoding

          //Step 1 read length
          int nb = input.readInt();
          byte[] digit = new byte[nb];
          //Step 2 read byte
          for(int i = 0; i < nb; i++){
            digit[i] = input.readByte();
          }
          String st = new String(digit);
      System.out.println("Received: "+ st);
    } else {
        WriteOut("Failed to connect to the server: "+Server+ "No Server");
    }
    }
    catch (UnknownHostException e){ 
        WriteOut("Sock:"+e.getMessage());
    }
    catch (EOFException e){
        WriteOut("EOF:"+e.getMessage()); 
    }
    catch (IOException e){
        WriteOut("IO:"+e.getMessage());
    }
}

The above gets the username and password and sends it as I explained before.

Then the Server does it's magic (or lack there of) The Server code:

//Step 1 read length
          int nb = input.readInt();
          byte[] digit = new byte[nb];
          //Step 2 read byte
           String st =null;
          for(int i = 0; i < nb; i++){
            digit[i] = input.readByte();

           st = new String(digit);
          }
          ServerOut("Recieved : " + ANSI_CYAN +
            clientSocket.getInetAddress() + ANSI_RESET + " - " + st);
            if (st.startsWith("loginreq ")){ //login attempt
                ServerOut("[" + Server.getTimeNow() + "] "+getThreadInfoString() +" Proccessing login request from: " + clientSocket.getInetAddress());
                try(BufferedReader br = new BufferedReader(new FileReader("users.txt"))) {
                    st = st.replaceFirst("loginreq ", "");
                    for(String line; (line = br.readLine()) != null; ) {
                        if(st.equals(line)){
                            String[] userDat = st.split(":");
                            boolean duplicate = false;
                            if(!connected.isEmpty()){
                            for(User u : connected){
                                if (u.getUsername().equals(userDat[0])){
                                    duplicate = true;
                                    break;
                                } else {

                                }
                            }
                            }
                            if (duplicate){
                                ServerOut("[" + Server.getTimeNow() + "] "+getThreadInfoString() +" User rejected (duplicate) " + userDat[0] + " : " + clientSocket.getInetAddress());
                                clientReturn("User was already logged in!");
                            } else {
                                User user = new User(clientSocket.getInetAddress().toString(), userDat[0], (int) time);
                                connected.add(user);
                                ServerOut("[" + Server.getTimeNow() + "] "+getThreadInfoString() + " User: '" + userDat[0] + "' logged in with ip: " + clientSocket.getInetAddress());
                                clientReturn("Login accepted!");
                            }
                            break;
                        }
                    }
                } catch (Exception ex){
                    clientReturn("Server ran into an error!");
                    ex.printStackTrace();
                }
            }

This reads the client input and if it starts with "loginreq ", then removes that and separates the string so that userDat[0] is the username and userDat[1] is the password. By this point it has already verified that the user exists. Then, if there are users connected (I've just started the server so there aren't any) it finds out if that same user is already logged in, which for some reason seems to return true. It then returns to the client that the user was already logged in. Not only is this a problem, but While the initial startup code using ServerOut prints to the console properly, now that it's in a new thread it seems to not print at all. I suspect this may be because I am using the Jansi console but IDK.

Sorry this was such a long post, but thanks for putting up with it!

Independent of your actual problem, the first thing I observe about your code is that you are ignoring the normal Java style rules about identifiers. Please don't do this ... if you want other people to read your code. — Stephen C, May 02 '16 at 02:57
I also noticed 2 security problems. 1) You are sending user names and passwords in the clear over what appears to be a non-secure (e.g. not SSL) network connection. 2) You are storing passwords in a file. — Stephen C, May 02 '16 at 03:09
I am aware of the current security flaws, and I plan to fix them. But for the time being, they will remain this way as I'm only testing on a local system — parabolah, May 02 '16 at 03:12

score 0 · Accepted Answer · edited May 23 '17 at 12:31

The problem appears to be that the connected list contains entries when it should not contain any.

You need to figure out how that has happened. Look at the code that creates the list object, and check that you have not prepopulated it with the wrong stuff. A list will not magically acquire entries out of thin air¹. If it is non-empty, then your code has put the entries there ... somewhere, somehow.

I would also add some logging to see what connected contains, and what userDat[0] is immediately before the "duplicate checking" code.

Finally, since you mention that this code is multi-threaded:

Check that there is only one connected and that all threads share it.
Check that you are synchronizing it properly.

(I suspect that you aren't synchronizing properly. That would not directly explain the behavior you have described ... connected non-empty before anyone has tried to login ... but it is liable to cause other problems.)

If this doesn't help, then you will need to provide an MCVE so that we can >>run<< your code to figure out what is actually going on here.

^{1 - While it is theoretically possible, I would discount the possibility of bugs in the class libraries or the JVM.}

Logging works fine provided I user system.out instead of the AnsiConsole.out in Jansi. The connected array is created outside of the thread independent code and works fine in an application with the same programming and it's not pre-populated and is completely empty. The two programs have only two differences there is 1. The fact that I'm using Jansi which most likely changes nothing, and 2. my Java version. Therefore, I'm left to assume that it is in fact Java which is causing the problem. — parabolah, May 03 '16 at 02:25
I've now changed my Java version and the program is working fine. — parabolah, May 03 '16 at 02:26
While changing Java versions might "fix" the problem, that does not mean that Java itself is at fault. It is (IMO) more likely that it is a fault in your code or your code's dependencies; i.e. something is making assumptions and/or depending on undocumented platform specific behavior. — Stephen C, May 03 '16 at 06:35

Comparing strings not working, and empty list not empty

1 Answers1