Manually parsing HTTP response header

Question

Due to some constraints I had to manually parse an HTTP response coming out of a stream.

I could not use buffered reader or scanner because the perform buffering and end up buffering bytes beyond the response header itself.

I'll be hitting my AWS S3 and only expect 200 or 206 status response.

This seems to work, but I'd appreciate some review.

final InputStream reader = socket.getInputStream();
stringBuilder.setLength(0);
//first fetch header
while (true) {

    final char read = (char) reader.read();
    if (read < 1) {
        //fail
        MiscUtils.closeAndIgnore(socket, reader);
        return false;
    }

    stringBuilder.append(read);
    if (read == '\n')
        break; //status header fetched
}

//check status
final String status = stringBuilder.toString();
Log.i("Downloader", status);
if (!(status.contains("200") || status.contains("206"))) {
    //fail
    MiscUtils.closeAndIgnore(socket, reader);
    return false;
}

//now consume the header
stringBuilder.setLength(0);
while (true) {

    final char read = (char) reader.read();
    if (read < 1) {
        MiscUtils.closeAndIgnore(socket, reader);
        return false;
    }

    stringBuilder.append(read);
    final int currentCount = stringBuilder.length();
    if (currentCount > 4 &&
            stringBuilder.charAt(currentCount - 1) == '\n' &&
            stringBuilder.charAt(currentCount - 2) == '\r' &&
            stringBuilder.charAt(currentCount - 3) == '\n' &&
            stringBuilder.charAt(currentCount - 4) == '\r') {
        break; //response consumed
    }
}

//print the header
final String completeResponse = stringBuilder.toString();
Log.i("Downloader", completeResponse);

Answer 1

This is a bit surprising:

stringBuilder.setLength(0);

Seeing the posted code starting with this is fishy, as it suggests that there is more code before it, and that the method is not decomposed enough, and should be multiple methods instead.

---

This is a bit surprising:

    final char read = (char) reader.read();
    if (read < 1) {

The read() method returns -1 in case of EOF. I suggest to change this check to read < 0 or read == EOF to avoid making reviewers ponder.

---

        MiscUtils.closeAndIgnore(socket, reader);

A common name for this kind of thing is closeQuietly, usually in a utility class called IOUtils (in Apache commons, for example). I suggest to follow that good example. MiscUtils is a bad idea.

---

    if (read == '\n')
        break; //status header fetched

It's recommended to use braces always, even with single-statement expressions like this.

---

if (!(status.contains("200") || status.contains("206"))) {

I'd go for the more strict and compact regular expression check:

if (!status.matches("HTTP/.* 20[06] .*")) {

---

It would be good to split the posted code to two methods:

• A method that consumes the status line (the first line)
• A method that consumes the remaining lines