File server redirects not handled by internal middleware

[dusty]

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

Caddy 0.10.6

2. What are you trying to do?

Protect pages using the internal directive. An app server responds with X-Accel-Redirect

3. What is your entire Caddyfile?

http://localhost:4000 {
  errors stderr
  log stdout
  root public
  internal /private
  proxy /secure localhost:3000 {
    transparent
  }
}

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

$ caddy

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

> curl http://localhost:4000/secure/index.html -v
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 4000 (#0)
> GET /secure/index.html HTTP/1.1
> Host: localhost:4000
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 301 Moved Permanently
< Date: Mon, 07 Aug 2017 16:02:55 GMT
< Location: http://localhost:3000/private/
< Server: Caddy
< Content-Length: 65
< Content-Type: text/html; charset=utf-8
<
<a href="http://localhost:3000/private/">Moved Permanently</a>.

* Connection #0 to host localhost left intact

6. What did you expect to see?

File public/private/index.html served

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

Returns redirect to http://localhost:3000/private/

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

Clone this repository

https://github.com/dusty/caddy-issue

npm install
npm start
caddy

[dusty]

Wanted to make a note on this.

It appears the trouble I'm seeing with the internal is a byproduct of something Caddy is doing to perhaps enforce clean urls. For example, check out the static scenarios in the readme at https://github.com/dusty/caddy-issue

On the simplest setup, where you are just statically serving a file if its named something that matches an index, and you request the file directly, instead of serving the file it will redirect you with a 301 to a URL without that filename.

eg: get /index.html redirects you to /

That is likely not going to be a problem, aside from an extra http request.

However, I am seeing a potential problem (unless I have a mistake in my configuration).

If you have a file named notindex.html, you will get redirected to /not. It appears Caddy is just stripping away the index from the end of path and sending a redirect with that. This is going to be a problem as you will likely get a 404 error, or get served incorrect content if you happen to have something located at not/index.html

It appears this behavior is what is causing the issue I was seeing with the internal.

[dusty]

I've never looked at Go code before, so I thought this might be interesting to look at. I think I located the part of the code that would send a redirect on notindex.html to /not. I added a failing test and then fixed it here:

https://github.com/dusty/caddy

Basically the check to see if an index file was called was simply using hasSuffix on the path. I'm thinking that was probably a shortcut to deal with slashes (eg: /something/index.html). I changed it to split on / and then grab the last element and compare that directly to the index we were checking.

For example
/something/notindex.html

notindex.html == index.html

instead of

strings.hasSuffix('/something/notindex.html', 'index.html')

Not sure if this will fix the internal piece or broke anything else. Will look at that as soon as I figure out how to install all the deps.

I installed go via homebrew and get this error. Any tips?

go install: no install location for directory /Users/dusty/tmp/caddy/caddy outside GOPATH
For more details see: 'go help gopath'

[mholt]

@dusty Nice job investigating! All your Go packages should be in your GOPATH (a "workspace"): https://golang.org/doc/code.html#Organization

The default GOPATH is $HOME/go, I think. So check that folder. Having things in $HOME/tmp won't work for you unless you set that to your $GOPATH env variable.

Caddy's static file server redirects requests to index files to their canonical path: /index.html -> /. This comes from within the staticfiles package, as you have found. And indeed, that might be a bug... we should be checking if path.Base(...) == indexFile (or something like that), rather than doing a naive HasSuffix() check on the whole path.

So your fix is on the right track, but consider using path.Base instead. Does that help?

[dusty]

Thanks! That is very helpful for a major go newbie. I'm currently googling things like "how to split a string in go" and "How to get the last element in an array in go" :)

[dusty]

Ok, so I pushed out the change you suggested, much cleaner. This took care of a file named somethingindex.html being redirected to /something. It turns out this in particular is a different issue than the one I created this for.

The redirect issue I am experiencing on serving an internal file named something defined as an index (index.html) still has an issue. I'll try digging for this tomorrow, any tips would be helpful.

eg:

GET http://localhost:4000/secure/index.html
301 redirects to:
http://localhost:3000/private/

However, /private is protected by the internal command. In addition the actual server you are proxying to may be on an internal network that can't be reached and shouldn't be presented.

To continue with the clean url behavior, I think it should be:
http://localhost:4000/secure/
Assuming the internal component should pick up on index files as well.

[mholt]

@dusty Nice work on the index file bug; that is indeed separate, go ahead and submit a PR for review.

I like @slightfoot's suggestion (in our Slack), where we possibly handle the redirect case within the internal directive. Similar to what we do now, but we just look at a different status code and response header. :)

[magikstm]

Original issue was fixed in #1812. Could this be closed?