Modifying HTTP headers in a piped stream

Note: This is the first time I've written anything whatsoever in straight C. In other words: I have no idea what I'm doing.

I recently had a task that involved temporarily relaying responses from one server, through a web-facing server, and then on to the client. Along the way, the relay server had to dynamically add an HTTP header of its own to the relayed response. Came up with a quick and dirty scripted solution, but it worked for the duration it had to.

Still, I thought it might be a good exercise for getting to know a little C, just for fun, since something like this would benefit from being written closer to the metal.

So below is a program (I call it headshape for lack of a better name) where you pipe in (say, from a curl command) an HTTP response including headers, and the program will add/remove/modify one of those headers, and pass on the rest on as-is. Given no arguments, stdin will just pass to stdout.

E.g.

$ curl -i example.com | headshape              # pass through
$ curl -i example.com | headshape Server       # remove the Server header
$ curl -i example.com | headshape Via 'foobar' # add/modify the Via header

So it does a bit of HTTP parsing to find the right header - or add it if it's not there - and to figure out what the response code is. Only responses with a 2xx status will be altered, 1xx responses will be ignored, and 3xx and above will cause it to just forward all remaining data unaltered (this partly an arbitrary choice; it's just a code exercise after all). It also defaults to forwarding everything as-is if it sees something it can't make sense of.

Below is the code. Much to my surprise it seems to work exactly as intended but, again, I've never written C code, so it's no doubt horrible. Any and all tips are welcome.

```
#include
#include
#include
#include

#define CRLF "\r\n" // HTTP line ending
#define BLOCK_SIZE 1024 // Any reason to make this larger/smaller?
#define HEADER_DELIM ": " // Used

A few items:
Use STDIN_FILENO

Instead of fileno(stdin) you can use the preprocessor symbol STDIN_FILENO, defined in ` which you have already included.
should_parse should be declared bool

Assuming you're using a compiler that isn't decades old (bool was added in the c99 standard), you can include and use bool for the type of should_parse. It also allows you to use the values of true and false within the code to make the intention of that flag a little more explicit.

The same is true for line_continued, the return value of isBoundary, and isDelimited and the second argument of isDelimited.
target_header should be const and argv should not

Your target_header variable should be declared const because its contents are not altered by the program. If you make that change, you can also remove the cast from the line

target_header = (const char *)argv[1];

However, you wouldn't actually need that cast anyway if you had declared main as:

int main(int argc, char *argv[])

It does seem like that ought to be const *argv but that's counter to what the C standard says. In section 5.1.2.2.1, it says that:

the strings pointed to by the argv array shall be modifiable by the program

which implies that they're not const.
Don't abuse fflush

You probably don't need to call fflush after every fwrite. The stream will automatically flush when the file is closed, which also happens automatically when main ends.
Make loop exits explicit

In passthru(), instead of using while(1) and then using a break it would be more clear to write it like this:

size_t bytes;
do {
    bytes = fread(buffer, sizeof(char), BLOCK_SIZE, stdin);
    fwrite(buffer, sizeof(char), bytes, stdout);
} while (bytes == BLOCK_SIZE && !feof(stdin));

Simplify isBoundary

The isBoundary routine can be simplified as this:

// is this line a "blank" CRLF line?
static bool isBoundary(char* line) {
    return strcmp(line, CRLF) == 0;
}

There is no need to also compare their lengths, since that's already implicit in strcmp.
Calculation of payload_line is complex

The payload_line variable size is calculated, and then malloc'd and then copied, but it's only used a few other places. What you've got isn't wrong, but it might be worthwhile instead allocating a fixed size and then using snprintf to populate the string. That would collapse the dozen lines used for that calculation to the much simpler single line:

snprintf(payload_line, BLOCK_SIZE, "%s" HEADER_DELIM "%s" CRLF, target_header, argv[2]);

I'm sure there's more but I lack the time at the moment.
Update

I found some time and a few other things in the code.
Refactor parseStatus

The current parseStatus routine does more work than needed to extract the status code. You don't need to copy the string to pass it to atoi so it can be greatly simplified like so:

static int parseStatus(char* line) {
    char* http  = strstr(line, "HTTP/1.");
    if (http) {
        http = strchr(http, ' ');
        if (http) {
            return atoi(http);
        }
    }
    return 0;
}

Refactor getHeaderName

Right now, the getHeaderName function is outlined something like this:

static char* getHeaderName(char* line) {
    static char header[MAX_HEADER_LENGTH];
    /* ... */
    return header;
}

This works (and even some ancient library functions were done this way) but it's not thread-safe and the design can easily be improved. Instead, it's better to have the calling function allocate a buffer and pass it in, along with the length rather than pointing to an internal static buffer.
Refactor isDelimited

In a similar vein, your isDelimited function is really only checking for two particular bytes, so it would be more clear and slightly faster to check for them directly instead of constructing a copy in memory and then using strcmp`.

StackExchange Code Review Q#48937, answer score: 10