Caddy: Extra " in Caddyfile log directive causes hosts to be skipped

[tobya]

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

0.10.0

2. What are you trying to do?

Run all my sites

3. What is your entire Caddyfile?

recipes.cookingisfun.ie {

  tls {
   protocols tls1.0 tls1.2
  }
    log / recipesx.log  "{remote} - [{when}] "{method} {path} {>Caddy-Rewrite-Original-URI} {proto} {status} {port}" {scheme} {mitm} " 


    errors  errorsrecipesx.log   
      proxy / http://recipes.cookingisfun.ie:3032

}

recipesmove.bcs.cookingisfun.ie {

  tls {
   protocols tls1.0 tls1.2
  }
    log / recipesmovex.log "{remote} - [{when}] "{method} {path} {>Caddy-Rewrite-Original-URI} {proto} {status} {port}" {scheme} {mitm} "   


    errors  errorsrecipesmovex.log   


    fastcgi / 127.0.0.1:35454 php
    root "C:/BCSSERVER/Websites/Recipes.cookingisfun.ie/api_v1.0/recipes/"
    startup "c:\php\php56\php-cgi.exe -b 35454" & 




}

another.bcs.cookingisfun.ie {
  root "C:/BCSSERVER/Websites/Recipes.cookingisfun.ie/api_v1.0/recipes/"
}

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

caddy -log stdout

5. Please paste any relevant HTTP request(s) here.

(paste curl command, or full HTTP request including headers and body, here)

6. What did you expect to see?

All my sites served

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

Only http/https:// recipes.cookingisfun.ie and http/https:// another.bcs.cookingisfun.ie are served. recipesmove.bcs.cookingisfun.ie is not.

8. How can someone who is starting from scratch reproduce the bug as minimally as possible?

Use the caddyfile with your own domains.

Details

I came across this bug while modifying my log string. I was copying and pasting and accidentally copied in a " inside a log string

log / recipesx.log "{remote} - [{when}] "{method} {path} {>Caddy-Rewrite-Original-URI} {proto} {status} {port}" {scheme} {mitm} "

note the " before {method}

When caddy is started there is no error it just drops the next domain recipesmove.bcs.cookingisfun.ie from being served. The domain after that another.bcs.cookingisfun.ie is served correctly. No Error is given.

This is in some respects user error as " is not allowed in a string that starts with a " however caddy should raise an error or have some mechanism to prevent dropping of hosts.

The following fixes it

log / recipesx.log "{remote} - [{when}] '{method} {path} {>Caddy-Rewrite-Original-URI} {proto} {status} {port}' {scheme} {mitm} "

[mholt]

Hey Toby, thanks for reporting this. Just FYI, from the Caddyfile docs:

A token is a sequence of whitespace-delimited characters in the Caddyfile. A token that starts with quotes " is read literally (including whitespace) until the next instance of quotes " that is not escaped. Quote literals may be escaped with a backslash like so: \". Only quotes are escapable.

I think we should make sure an error is returned. log should ensure that only a certain number of arguments are given, and/or the lexer should make sure that the file doesn't end in open quotes.

Any takers? This shouldn't be too difficult. Preferably those who are new to the project!

[francislavoie]

Crazy idea, but would it make sense for directives like this, where the last argument is a string, to just take the entire argument literally? I.e. read everything up to the first newline or { as the string, and for backwards compatibility, trim the " on either side. That would mean that an additional argument after that one would be impossible, if a new one would be added in the future. I think the Caddyfile would read better that way and be a bit easier to use.

The recent PR allowing affixes around the presets from last week did help with this a good amount, but for fully custom log formats, it's a bit gross to deal with unfortunately.

[cez81]

May I have a look at this one?

[tobya]

@cez81 Please do, we would be delighted. Just ask questions here if you have any. Thanks.

[mholt]

@francislavoie I do not think I like the inconsistency that introduces. Let's leave the Caddyfile as-is, but just fix the log syntax parser and the lexer.

[cez81]

I have added a check for number of arguments to start with. Do you think that will be enough or should have a look at the lexer too?

[tobya]

This is fixed by #1672