Quickly throwing together a text-and-button-based client-server web game

Question

With much of the world in lockdown at the moment, my friends and I wanted a way to play our favourite game, Celebrity, over video chat.

This seemed like a fun coding project, so I put together a little web app. The functionality it would need to provide would be the ability for everyone to put names into a virtual hat, and for everyone to then take turns drawing names from the hat at random.

My experience is in desktop application programming, and I have essentially none in web development. My goal was to get something minimally functional as quickly as possible, with no thought given to code elegance or maintainability, and to learn a little bit along the way.

I settled on a pattern which was sufficient for this goal, but is unlikely to scale well for larger projects. This pattern is illustrated at the bottom of the post, together with my main questions, but I have some more specific minor questions along the way.

The complete code is on github, but hopefully there's enough detail in this post for you to answer my questions without looking at all of it.

Some decisions I took straight away:

• The game would run on the server, so that there would be a single authority on the game's current state
• The server would be written in Java, since it's the language I'm most familiar with
• The client would use HTML forms + buttons, and Javascript, since my background knowledge suggested this would be the quickest and simplest path to take.

And the key bits of knowledge I was missing:

• How do I set up an HTTP server?
• How do I send data back and forth between browser and server, such as the celebrity names, whose turn it is, when a player has started/ended their turn, and what a player is doing on their turn?
• How do I create a webpage whose content changes dynamically in response to user actions or messages from the server?
• How do I keep track of individual clients and their sessions?

Setting up an HTTP Server

A bit of Googling told me there were many ways to do this, and I was a bit overwhelmed by choice. It looks like probably the most standard way to do something robust and scaleable would be to use Apache, but I saw some suggestions that this could be overkill for simple projects, and I didn't get as far as learning how Apache would run my own code. In the end, I went with the solution in this answer, using the com.sun.net.httpserver.HttpServer class, since the Hello World example in that answer worked immediately.

What would be the disadvantages of implementing a larger and feature-richer server using this class?

I found I had to write my own code to find the HTML and Javascript files, map their names to URLs the users could access, and read these files and send their content when the associated URL was requested. Is this normal? Isn't there a way to just say "serve all files in this directory"?

Client-Server Communication

It quickly became clear that the standard way to send data from client to server is to create an XMLHttpRequest in Javascript, and send it using a GET or POST request. All my client->server communication was handled this way, except for submission of forms, since the form has a built-in mechanism to submit GET or POST requests.

After looking at some examples, I settled on the following pattern, which was sufficient to achieve what I wanted. The client sent a request like so (Javascript):

xmlhttprequest.open("POST", "test", true);
xmlhttprequest.send("param=paramValue&param2=paramValue2");

And the server could receive this with code like the below (Java):

httpServer.createContext( "/test", new HttpHandler() {
    public void handle(HttpExchange aExchange) throws IOException {
    // get the text "param=paramValue&param2=paramValue2" from aExchange.getResponseBody()
    // process it as required
    // send a response as text
}
} );

The response text was then set to the responseText field of the XMLHttpRequest, and again could be processed as required. The response text was also sent as an &-separated list of key-value pairs.

What I found strange about this was that there was no hard requirement to use XML! I always got errors in the Javascript console that the response text was not well-formed XML, but this didn't break anything. But I assume there must be something I've missed that would mean I could avoid parsing the strings myself.

Aren't there Javascript functions and Java methods for creating and reading tree-like or graph-like data structures, which can be written/read as XML under the hood, without the programmer needing to process the message content directly?

Sometimes I found that the XMLHttpRequest wasn't sent successfully unless I added a 0.5s wait using setTimeout, before sending the request. I can do more testing and get more detail if necessary, but does anyone know what could be going on there?

For communication server->client, without any initial request from the client, I didn't find a solution. So I settled on each client requesting the entire state of the game every half a second. Since the game state was very small (less than 1 kb in text form), this was sufficient.

What's a better way for the server to send data to the client without any request from the client?

Dynamically Updating The Page

I found two ways to do this, and used both liberally:

1. Update the innerHTML field of a document element
2. Lots of hidden divs which could be unhidden as needed

I came to prefer the second option, since the final HTML is easier to read - you can see everything that can potentially be displayed. In the future, I would only use the first option for content that isn't known at write-time.

Are there any disadvantages to this approach?

I can see that if I want to keep certain content initially hidden from users, I'd have to avoid this, as they could see it using View Source. But if they can read Javascript, they could also see anything I'm going to set to an innerHTML field using a script. So if I really wanted to keep it hidden, I guess I'd have to generate it server-side. Anyway, that wasn't a concern for this game.

Session Handling

I used a very simple solution which probably doesn't count as creating a true session. There is a single HTML page to load, and the server dynamically modifies it by inserting some code to set a cookie with the name-value pair session=<a randomly chosed UUID>. I found that each further request to the server automatically included this cookie, so I could keep track of identities.

Is that all there is to it?

Putting it all together: Ctrl-F Oriented Programming

The below code snippets illustrate the pattern used throughout the application. They handle the pass function, in which a user gives up on getting their team to guess the current name, puts the name back in the hat, and draws another one. There's a button defined in HTML, a Javascript function which sends a request to the server to say that the user is passing on a given index in the list of names, a handler in the server to re-shuffle the names and return the new list to the client, and an inner function in the client to handle this response.

The main problem I have with this pattern is that it uses a lot of "Ctrl-F oriented programming". All the following strings need to be kept identical in multiple places, with no automatic check for errors, meaning I need to do a lot of manual checking of the files:

• The button ID "passButton"
• The Javascript function name "pass"
• The string "pass" used as the second arg to XMLHttpRequest.open and the string "/pass" used as the first arg to HttpServer.createContext
• The parameter "passNameIndex" used in the POST request
• The parameter "nameList" used in the response to the POST request

In such a small application, this isn't a major problem. In a larger one it would surely become a nightmare.

Are there tools to handle this kind of problem? For example, ways to define tightly-coupled client and server code in the same file (e.g. if the server code was also written in Javascript)? Or ways to define such strings in a separate file used by both the server and the client, with a compile-time check that only pre-defined strings are used?

Code snippets follow. HTML defines the button:

<button id="passButton" onclick="pass()">Pass</button>

Javascript sends the request and processes the response:

function pass() {
    document.getElementById("passButton").style.display = 'none' // hide button while processing the pass
    setTimeout( function() {                                     // timeout mysteriously needed for request to work
        var xhttp = new XMLHttpRequest();
        xhttp.onreadystatechange = function() {
            if (this.readyState == 4 && this.status == 200) {
                document.getElementById("passButton").style.display = 'block'   // restore button

                // The game maintains a shuffled list of celebrity names, which is modified in response to the pass.
                // Here we process the response by updating the client's shuffled name list to hold the list provided by the server
                var arr = toAssocArr(this.responseText)
                var nameListString = arr["nameList"];
                if ( nameListString != null ) {
                    nameList = nameListString.split(",");
                    updateCurrentNameDiv();
                }
            }
        }

        // send request
        xhttp.onload = function() {}
        xhttp.open("POST", "pass", true);
        xhttp.send("passNameIndex=" + current_name_index);

    }, 500 );
}

Java processes the request and sends the response:

server.createContext( "/pass", new HttpHandler() {
    
    @Override
    protected void handle(HttpExchange aExchange) throws IOException {

        // From cookie, get Session, then Player, then Game
        String sessionID = HttpExchangeUtil.getSessionID(aExchange);
        Session session = SessionManager.getSession(sessionID);
        if ( session != null ) {
            Player player = session.getPlayer();
            Game game = player.getGame();

            // Convert input string like "passNameIndex=5" to a LinkedHashMap
            LinkedHashMap<String, String> requestBody = HttpExchangeUtil.getRequestBodyAsMap(aExchange);
            String passNameIndexString = requestBody.get("passNameIndex");
            if ( passNameIndexString != null ) {

                // parse the provided passNameIndex, process it, and send the new shuffled name list as a response
                try {
                    int passNameIndex = Integer.parseInt(passNameIndexString);
                    game.setPassOnNameIndex( passNameIndex );
                    
                    sendResponse(aExchange, HTTPResponseConstants.OK, "nameList=" + String.join(",", game.getShuffledNameList()));
                }
                catch ( NumberFormatException e ) {
                    e.printStackTrace();
                }
                
            }
        }

        // No error handling for if session/player/game isn't found, can deal with that later
    }
} );

As I said, essentially the whole application followed this pattern. My main questions are:

1. For experienced web developers with a similar goal (i.e. something quick 'n' dirty which just manages to work), what would you have done similarly or differently?
2. If you were aiming for something bigger, like a long-term project which would scale to a large number of users, and with new features continually being added, what would you have done differently?

Miscellaneous Questions

If I go bigger on my next project, what security issues do I need to consider? As far as I can see, there's no possibility of code injection via the HTML forms, because the input is encoded by default, but have I missed something?

The complete code is on github. To answer the misc question, you probably just need to check the headers and forms in celebrity.html.

Answer 1

Aren't there Javascript functions and Java methods for creating and reading tree-like or graph-like data structures, which can be written/read as XML under the hood, without the programmer needing to process the message content directly?

When communicating between the client and server, the de-facto standard format to use for data structures is JSON. It's much more concise than XML, is easily parseable (on both ends), and is easily created from an object (on both ends). That is, on the client-side, rather than

game_arr = toAssocArr(this.responseText)

You'd like to be able to do

game_arr = JSON.parse(this.responseText);

You'd also like the sent response to contain the data as you want it, without requiring further Javascript processing. For example, you wouldn't want to have to do

var nameListString = game_arr["nameList"]
if ( nameListString != null ) {
  nameList = nameListString.split(",")
}

Preferably, the nameList property would already be an array of strings after you JSON.parse it, rather than a string you have to split later. On the Java side of things, there are various ways to build the JSON that is required. Once it's built, just call sendResponse with the JSON string, and the JSON.parse on the client-side will transform it into a Javascript object.

If you want your script to use modern web standards, consider using the Fetch API instead of XMLHttpRequest. fetch is a lot more concise, and uses Promises, which are usually a lot easier to work with than callbacks, especially when you have multiple asynchronous actions. For example, rather than

function updateGameState(gameID) {
  var xhttp = new XMLHttpRequest();
  xhttp.onreadystatechange = function() {
    if (this.readyState == 4 && this.status == 200) {
      game_arr = toAssocArr(this.responseText)
      // 150 more lines of code
    }
  }

  xhttp.open("POST", "requestGameState", true);
  xhttp.send("gameID=" + gameID);
}

you could use

function updateGameState(gameID) {
  fetch('requestGameState', { method: 'POST', body: 'gameID=' + gameID });
    .then(res => res.json()) // this will automatically call JSON.parse
    .then((result) => {
      gameArr = result;
      // more code here
    })
    .catch((err) => {
      // don't forget to handle network/server errors here!
      // better to gracefully degrade than to fail silently
    });

The // 150 more lines of code is a bit smelly - consider splitting it up into separate functions. Consider implementing the Single Responsibility Principle. For a reasonably-sized script (more than 200 lines of code or so), I highly recommend using modules instead - this allows you to split up hundreds of lines of code into smaller self-contained script files, which make things a lot easier to understand once you're past the initial hurdle of figuring out how they work.

(like always, if you want ancient obsolete browsers to be able to run your code as well, use a polyfill)

What's a better way for the server to send data to the client without any request from the client?

You could use websockets instead, which allow for the server to communicate to the client whenever the server wishes, after the client has made an initial request. On the client-side, this could look like:

const socket = new WebSocket('gamestate');
socket.addEventListener('message', (event) => {
  // process event.data, assign to gameArr
});

Note that your gameArr (or game_arr) isn't actually an array - it's a plain object. An array is an ordered collection of values. An object is a collection of key-value pairs, which is what you're working with. A less misleading variable name might be gameState.

By far, the most common variable naming convention used in Javascript is camelCase for nearly everything (except classes). You're using both camelCase and snake_case in your script; you might consider deciding on one convention, and then sticking to it. Once you have a standard, that's one less thing to have to keep in mind while writing or debugging. You may also consider using a linter to enforce code styles - not only for variable names, but for other problems which are likely to lead to bugs and harder-to-read code (such as missing semicolons, using bracket notation obj["prop"] instead of dot notation obj.prop, using loose equality comparison == instead of strict equality comparison ===, etc. There are a large amount of potential improvements that could be made on these fronts.)

Regarding control-F programming:

I wouldn't worry too much about having to make sure the nameList property being parsed in JS matches the property sent in Java. You need some sort of API standard regardless; it's not a code smell, there's no other choice, given that the client and server are completely separate mediums.

The button ID "passButton"

In order to not repeat it more than necessary, save it in a variable, then reference that variable instead of selecting the element again. You could also consider not using IDs at all - they create global variables, which can result in bugs. Consider using classes instead. For example, if the button has a passButton class:

const passButton = document.querySelector('.passButton');
passButton.style.display = 'none';
// later, reference passButton instead of calling document.querySelector again

The Javascript function name "pass"

This is a problem, and only partially for the reason you said. Inline handlers have too many problems to be worth using; they have a demented scope chain, require global pollution, and have quote escaping issues. Use addEventListener instead. Once you have a reference to the button with querySelector, you can do:

passButton.addEventListener('click', () => {
  // put all the code that used to be inside the "pass" function here
});

(and, of course, remove the onclick="pass()" from the HTML. Best to do the same for all your other inline handlers, you have many of them.)

The parameter "passNameIndex" used in the POST request

Since the name of the endpoint already indicates what the value being sent is, why not just send the plain value?

xhttp.send(current_name_index);

(or the equivalent with fetch)

Then, on the Java side, rather than getRequestBodyAsMap, just extract the plain request body as a string, and you have the passNameIndexString variable that you need.

what security issues do I need to consider?

The biggest issue I saw was this pattern, present in a few places:

htmlTeamList += "<h3>" + teamName + "</h3>\n" + "<ul>\n";
for ( var j=1; j<teamNameArr.length; j++) {
  htmlTeamList += "<li>" + teamNameArr[j] + "</li>\n"
}

Directly writing an HTML string by concatenating variables is a potential security hazard, unless you're absolutely certain that the input is trustworthy. Otherwise, it'll allow for arbitrary code execution, and user cookie information could be sent to a malicious actor. For example:

const teamName = `<img src onerror="alert('evil')">`;
const teamNameArr = [];

let htmlTeamList = '';
htmlTeamList += "<h3>" + teamName + "</h3>\n" + "<ul>\n";
for ( var j=1; j<teamNameArr.length; j++) {
  htmlTeamList += "<li>" + teamNameArr[j] + "</li>\n"
}

document.querySelector('div').innerHTML = htmlTeamList;

<div></div>

I think it would be good to get into the habit of either never concatenating an HTML string with variables, or of always escaping strings first (though this can easily bite you if you happen to forget to do it). (To sanitize, remove the < > brackets from strings before inserting them)

Answer 2

Welcome to Code Review. Here some suggestions for you :

LinkedHashMap<String, String> requestBody = HttpExchangeUtil.getRequestBodyAsMap(aExchange);

Declare requestBody as Map, if you can choose always the most generic interface.

try { //send your response 
} 
catch (NumberFormatException e) { 
  e.printStackTrace();
}

Usually requests are logged using for example Logger saving them in a log file, so you can proceed with exam of requests and see eventually what's gone wrong.

It seems me from your code you have already clear the main concepts about web programming like sessions, these are my personal answers to your questions based on my experience:

For experienced web developers with a similar goal (i.e. something quick 'n' dirty which just manages to work), what would you have done similarly or differently?

Normally web java projects are based on existing complex frameworks, for me the best option is always rely on which framework is used for the project and model my service using just framework libraries if it is possible. What happens if your code work perfectly on your pc and not work within the framework?

If you were aiming for something bigger, like a long-term project which would scale to a large number of users, and with new features continually being added, what would you have done differently?

Almost the same answer before, check which one well known framework has the characteristics you are looking for and use it from the start, avoid if possible all security problems and let the framework does its job.