caddyfile: Reject directives in the place of site addresses

[armadi1809]

Resolves #6095

@francislavoie from my understanding, that's the only change needed but I might be wrong. Also, let me know if you think we need more tests.

[francislavoie]

Yeah I think we should have a test that ensures http://handle is still allowed. I think it will be because directives aren't registered with a scheme, but just a sanity check.

Also, I'm realizing that we're not marking where in the Caddyfile (filename/linenum) the problem happened. I don't think that information is known at this point in the parser unfortunately. Maybe we should move these checks to be somewhere else where we still have the Tokens?

[armadi1809]

@francislavoie I get your point with the filename and line number but I am not entirely sure about where to move this new check since we need to loop through the block keys to determine if we are using a directive as a site address but we only have access to the Tokens at the segment level. Maybe I am missing something here though?

[francislavoie]

I think we could either rework ServerBlock to hold []Token instead of []string for the site addresses (to retain the file/line along with the text), or move the errors to func (p *parser) addresses() error { where it actually parses the addresses initially.

[armadi1809]

I think I like the second option better as it feels more intuitive to perform the check there. One issue is that we don't have access to the registeredDirectives in the caddyfile package which makes me wonder if the only solution is reworking ServerBlocks. Unless there is another way of accessing all the caddy known directives.

[francislavoie]

Ah right I forgot about that 😢 yeah it'll probably be better to pass down the tokens I guess. We can add some helper methods to ServerBlock to unwrap the tokens to a []string if necessary.

[francislavoie]

Tests are failing - looks like we need to add the "," trim in another spot.

[armadi1809]

@francislavoie I think we should be good to go now. I changed the location of that Trim to be where we actually add the token rather than in the helper that maps the tokens to strings. I also addressed your comments from above. Thanks for your feedback and help.

[armadi1809]

All the feedback items from above should now be resolved.

[francislavoie]

Thanks, this is great!

I made a few minor test adjustments, i.e. the error now asserts that the file/line is present, and I changed a couple other tests to fix some port conflicts that happened on my machine due to the tests using common ports (admin 2019 but I already have Caddy running, and 8080 is commonly used for some HTTP apps).