Caddy: ; At end of header directive causes invalid http headers

[nicolasazrak]

1. What version of Caddy are you running (caddy -version)?

Caddy 0.9.3

2. What are you trying to do?

Add a header

3. What is your entire Caddyfile?

caddy.test {
        tls off
        root /home/nico/apps/caddy
        header / {
                Test "max-age=1814400";
        }
}

Note the ; at the end of Test header

4. How did you run Caddy (give the full command and describe the execution environment)?

./caddy -conf Caddyfile

5. What did you expect to see?

HTTP/1.1 200 OK
Accept-Ranges: bytes
Content-Length: 0
Content-Type: text/html; charset=utf-8
Etag: W/"564e6bf5-0"
Last-Modified: Fri, 20 Nov 2015 00:40:21 GMT
Server: Caddy
Test: max-age=1814400
Date: Mon, 03 Oct 2016 18:23:39 GMT

6. What did you see instead (give full error messages and/or log)?

HTTP/1.1 200 OK
;: 
Accept-Ranges: bytes
Content-Length: 0
Content-Type: text/html; charset=utf-8
Etag: W/"564e6bf5-0"
Last-Modified: Fri, 20 Nov 2015 00:40:21 GMT
Server: Caddy
Test: max-age=1814400
Date: Mon, 03 Oct 2016 18:23:39 GMT

Note under HTTP/1.1 200 OK there is a line with ;:

7. How can someone who is starting from scratch reproduce this behavior as minimally as possible?

It can be reproduced adding the given caddyfile. It must give a 200 response, another one like 404 does not reproduce the issue.

[elcore]

Hello @nicolasazrak,

please change your Caddyfile to:

caddy.test {
        tls off
        root /home/nico/apps/caddy
        header / {
                Test "max-age=1814400;"
        }
}

[nicolasazrak]

After a long time debugging I realized that was the cause, but should not caddy show a waning when there is and invalid syntax?

[elcore]

After a long time debugging I realized that was the cause, but should not caddy show a waning when there is and invalid syntax?

@nicolasazrak that is a good question!

ping @mholt

[mholt]

This highlights something I wasn't sure about -- from a long time ago (almost 2 years now) when I first started building Caddy. One of the first parts I wrote was the Caddyfile parser, and although I knew quotes would be used to enclose strings, I didn't know how to split the tokens in this case:

"some string"s here  # how many tokens?

Even though it is obvious for (most of) these cases:

"some strings here"   # 1 token
"some strings" here   # 2 tokens
"some" strings here   # 3 tokens
som"e strings here    # 3 tokens
some" strings here    # 3 tokens
some" strings" here   # 3 tokens

Quotes complicate things, so only use them when needed - and you could take them out in this case:

Test max-age=1814400;

So I guess the question is, what are the tokens of this:

"some string"s here

Is it these:

A

some string
s
here

or

B

some strings
here

Right now we choose A.

[nicolasazrak]

That makes sense. Maybe header directive should only accept only one header per line.
I've found that if you write

header / {
    Test "max-age=1814400" Foo Bar
}

You get

HTTP/1.1 200 OK
Foo: Bar
Test: max-age=1814400

And the worst case is when you have an even number of tokens, it shows a header without a value.

[mholt]

It should, I agree, that one header per line should be enforced. But we also might have to tweak slightly how tokens are parsed which happens in the lexer, before the header directive sees any of the input. So probably two simple changes.

[wendigo]

I can work on header directive but need some guidance in the lexer.

[mholt]

@wendigo Great! Submit a PR and we'll work through it with you. Shouldn't be too difficult.

[wendigo]

I've been trying to fix that but I can't fully understand lexer and logic behind it. Need help of @mholt

[mholt]

@wendigo No problem -- what about just tweaking the header parsing logic, so it only accepts one header field per line?

[wendigo]

Here you go @mholt: https://github.com/mholt/caddy/pull/1280

[wendigo]

Fix was merged to the master.