ClientApp dereferences the response and the success key without checking either

[dmccoystephenson]

Problem

ClientApp.sendSquareRequest reads the server's reply along a path on which two separate values
are dereferenced without being checked, so a client whose server has gone away, or whose server
answered in a shape it did not expect, terminates on a NullPointerException instead of
reporting what happened.

The response string is not checked before being parsed.

// src/SimpleServer/client/ClientApp.java:41
receivedMessage.fromString(client.getStringFromServer());

getStringFromServer returns null on an IOException
(src/SimpleServer/client/Client.java:41), and in.readLine() itself returns null once the
stream reaches its end, which is what a closed or dropped connection produces. Message.fromString
then dereferences its argument on the first statement it executes:

// src/SimpleServer/Message.java:44
for (int i = 0; i < string.length(); i++) {

There is no null guard between the two.

The success key is not checked before being compared.

// src/SimpleServer/client/ClientApp.java:44
String success = (String) receivedMessage.get("success");

if (success.equals("true")) {

Message.get is a bare HashMap lookup (src/SimpleServer/Message.java:19) and returns null
for an absent key. Every response that Protocol composes does set success, so the reachable
route to this line holding null is a message that never arrived intact rather than one the
server chose to send.

The practical effect is that the failure a user is most likely to hit — starting ClientApp with
no server listening — is also the one reported worst. It is reached in combination with #10, which
records that the hostname is hardcoded to "Walter", and #11, which records that the constructor
discards the results of its initialize* helpers and leaves in and out null.

Evidence

Read out of src/SimpleServer/client/ClientApp.java, src/SimpleServer/client/Client.java and
src/SimpleServer/Message.java at d34c1da. No compiler was available in the session in which
this was filed, so no client was run against a dead server and the resulting exception was not
observed. The dereferences and the absence of any guard between them are stated from the source;
the exception they are expected to produce is UNVERIFIED.

Suggested fix

The response could be captured into a local, checked for null, and reported as a lost connection
before parsing; success could be compared with the literal on the left, or checked for null
first. The fix is confined to ClientApp and does not require the wire format to change.

Whoever takes this is asked to confirm the behaviour against a running server first, since #11
touches the same call path and the two are likely to be worth fixing together.

---

This issue was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

drafted by Claude on behalf of Daniel Stephenson