Simple High Score Server for HTML Game

I have implemented a basic high score server for my strategy game. The goal here was to get the high scores working for the browser based version of the game (I am using the libGDX library with GWT for this version). Unfortunately this required using websockets, which made the process more difficult than I would like.

I will (hopefully) one day extend this basic foundation into a full blown MMO version of the game.

The logical flow of what happens here is that the client opens a connection with the server, the server sends a welcome message, the client responds to that by sending its score, the server responds to that by sending its list of high scores, and when the client receives that it sends a success message and closes the connection.

Here is the server code:

```
public class Server {

private static class InnerWebServer extends WebSocketServer {

private final Server server;

public InnerWebServer(Server server, int port) {
super(new InetSocketAddress(port));
this.server = server;
}

@Override
public void onOpen(WebSocket conn, ClientHandshake handshake) {
System.out.println("Client opened connection " + conn.toString());
this.server.sendWelcome(conn);
}

@Override
public void onClose(WebSocket conn, int code, String reason, boolean remote) {
System.out.println("Client closed connection " + conn.toString());
}

@Override
public void onError(WebSocket conn, Exception ex) {
}

@Override
public void onMessage(WebSocket conn, String message) {
System.out.println("received = " + message);

if (message.equals("Success")) {
this.server.sendGoodbye(conn);
} else {

//filter out malicious stuff here
if (this.server.processScore(message)) {

try {
this.server.saveSc

My main concerns going forward are two fold: security and bandwidth
usage. Using an Amazon server means that I would have to pay by the
gigabyte for bandwidth, so I would like to reduce that as much as
possible. This means that a DDOS type attack could be very costly.

Wouldn't amazon handle ddos attacks for you?

And regarding security: currently it would be extremely easy for a player to submit any highscore they want. As you say that you have some more plans for this game, that might not be ideal.


Even a situation where a player opens and closes the high scores menu
repeatedly could get pretty costly. I have some plans in place to deal
with these issues, but any feedback or advice would be much
appreciated.

You could make players register, and require authentication when submitting/retrieving the highscore. Then you can limit the amount of retrieving.


Another big concern is that I am using a predetermined string
delimiter, which will mean I will pretty much have to prevent the user
from putting that string delimiter in their name when they create it.
This does not seem optimal.

It's not optimal, but I think that it is acceptable. You wouldn't even have to prevent the user from using those characters though, just split by the last occurrence (or from back to front if you have more than two values later on).

Printing Serverside

System.out.println on the server doesn't make that much sense. Nobody is going to read them, and they take up resources. I'm assuming that they are debug statements, but having so many of them makes them hard to handle (will you always comment all out when deploying, and then uncomment again when debugging?)

If you want to keep it simple, you could at least extract the printing to a separate method, so that you only have to change code in one place (then you could add an if based on a config file, comment/uncomment the print, or print to a file).

Error Handling

Why are you turning a perfectly fine exception in processScore into a boolean return value? Malformed input seems like an exceptional case, so it can be handled with exceptions.

Quite often, you catch an exception, print an error message, and that's it. That doesn't seem like a good idea. I would log the errors to a file, and I would inform the user about it (add a sendError method). Just silently failing will make debugging later on quite difficult and it will also be frustrating for a user if they are not informed about failures and possible reasons for them (is the input malformed, or does the server have problems? this is something a user would probably like to know).

onError should probably also log and inform the user.

And an invalid integer should also throw an exception, not set the score to 0.

Misc

• use TODO for stuff that is not yet done. filter out malicious stuff here sounds as if that is currently already happening, when it is not. Adding TODO will also make it a lot easier to find all todos and not forgetting any.

• I rarely see concat used, and I think that the reason is that the code doesn't look as good as with + or StringBuilder, and also performs worse than StringBuilder.

• extract static strings to static fields or configuration files (such as the file name which you have twice in your code, or the delimiter that also occurs more than once).

• this.socket = null; //not sure if this would be good or not: no, it's not needed.

StackExchange Code Review Q#85113, answer score: 3