Caddy: Bad Rewrite parameters break fastcgi matching

[tobya]

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

0.10 but also present in 0.9.5

2. What are you trying to do?

rewrite a url to a php page

3. What is your entire Caddyfile?

basil.kinchik.ie:80 {

  #breaks php fastcgi - renders as html
  rewrite /admin wp-admin/admin.php

  root  d:\caddy\webroot\basil

  fastcgi /  localhost:49249 php
  startup d:\php7\php-cgi.exe -b 49249 &
}

It works correctly if the rewrite rule is replaced with

 rewrite /admin /wp-admin/admin.php

note the / before /wp-admin/admin.php

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.

http://basil.kinchik.ie/admin

6. What did you expect to see?

http://basil.kinchik.ie/wp-admin/index.php executed as php and returned as html

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

http://basil.kinchik.ie/wp-admin/index.php rendered as text

eg.

<?php
 echo '<P>this is a php page';

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

Use the caddyfile provided. Ensure the file wp-admin/index.php exists and contains php code.

There should never be a situation where a php file is served without going through the php engine this is a security risk. It would be very easy for a configuration to slip through that caused this.

[tobya]

I found the place where the mismatching happens

https://github.com/mholt/caddy/blob/master/caddyhttp/fastcgi/fastcgi.go#L40

I will try to submit a PR

[mholt]

Thanks Toby! Be careful though, we just need to decide what rewriting a URI to wp-admin/admin.php really means (what's the final result given a base path of / vs /foo vs /foo/ etc)) and see if this is actually a bug in Caddy or just user error.

[tobya]

@mholt I am working on a very simple fix at the moment and will look at what happens when I check for a / at the base of any url being matched by fastcgi and add it if required.

An alternative would be to raise a syntax error on parsing the caddyfile if rewrite to destination does not begin with / (all examples do).

Definitely a bug though. If I have fastcgi set to serve php on / and it doesnt serve a file via php fastcgi, then that is a bug :smile:

[mholt]

What is the current behavior of that rewrite, though (without the leading /)? What does it rewrite the URI to?

[tobya]

rewrite /admin wp-admin/admin.php

this rewrites to wp-admin/admin.php as requested. However when it gets to fastcgi code it is matched against / which is doesn't match, but should. This leaves the request without a fastcgi match and so is served as a static file and not interpreted by php.

rewrite /admin /wp-admin/admin.php

Obviously works correctly because now fastcgi / will match wp-admin/admin.php with the leading /

There is 3 possible fixes I can think of

• Check and add / is at the begining of any path passed into fastcgi for matching, so / will always match anything.
• Make / mandatory (syntax error) at the begining of the To part of a rewrite clause.
• When match is / for fastcgi simply match anything, do not check!

[abiosoft]

My vote is for this

Make / mandatory (syntax error) at the begining of the To part of a rewrite clause.

[mholt]

Hm, yeah, I second Abiola's vote.

[tobya]

Does anyone have a prefernce for error msg? I can use standard syntax error

Syntax error: Unexpected token 'wp-admin/admin.php', expecting '/wp-admin/admin.php'

or my preference is to create a new error

Rewrite error: Rewrite path must begin with '/'. Provided: wp-admin/admin.php

[mholt]

Do the second one! Or if you still want to do a syntax error, just say specifically that the rewrite path must begin with /.

(Ignore any comment notifications you may have gotten from a user account that looked like mine earlier today.)