Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fileserver: Add status code override #4076

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

francislavoie
Copy link
Member

After reading a question about the handle_response feature of reverse_proxy, I realized that we didn't have a way of serving an arbitrary file with a status code other than 200. This is an issue in situations where you want to serve a custom error page in routes that are not errors, like the aforementioned handle_response, where you may want to retain the status code returned by the proxy but write a response with content from a file.

This feature is super simple, basically if a status code is configured (can be a status code number, or a placeholder string) then that status will be written out before serving the file - if we write the status code first, then the stdlib won't write its own (only the first HTTP status header wins).

@mholt
Copy link
Member

mholt commented Mar 29, 2021

@mholt mholt added the under review 🧐 Review is pending before merging label Mar 29, 2021
@francislavoie
Copy link
Member Author

francislavoie commented Mar 29, 2021

Unfortunately I don't think so, because I don't think error routes will let you keep the same error code if you're serving a file with file_server.

But either way, that would be pretty roundabout... reverse_proxy -> handle_response -> error -> handle_errors -> file_server? It would be better to cut out error and handle_errors and leave that to actually be for errors produced by Caddy.

This is still useful in other situations though, e.g. if you want to serve a "maintenance mode" page with a specific status, without emitting error, etc. Forcing users to use error -> handle_errors seems unwise, when they would just want a quick "respond with a file with X status code" based on some matcher decision.

@mholt
Copy link
Member

mholt commented Mar 29, 2021

Why did we add the errors directive then if we don't want people to use it? The whole point is to consolidate error handling.

@francislavoie
Copy link
Member Author

francislavoie commented Mar 29, 2021

I'm saying those are separate but valid usecases. The times where you use error are not necessarily the same as those you'd want to just serve a file with a specific status.

Here's an example of how it would look when paired with handle_response:

example.com {
	reverse_proxy localhost:8080 {
		@fourhundred status 4xx
		handle_response @fourhundred {
			root * /path/to/errors
			rewrite * /{http.reverse_proxy.status_code}.html
			file_server {
				status {http.reverse_proxy.status_code}
			}
		}
	}
}

But if it had to be done with error, it would look like this (way more complicated to reason about):

example.com {
	reverse_proxy localhost:8080 {
		@fourhundred status 4xx
		handle_response @fourhundred {
			error {http.reverse_proxy.status_text} {http.reverse_proxy.status_code}
		}
	}

	handle_errors {
		@fourhundred expression {http.error.status_code} >= 400 && {http.error.status_code} <= 499
		handle @fourhundred {
			root * /path/to/errors
			rewrite * /{http.error.status_code}.html
			file_server
		}
	}
}

And that assumes that file_server will still write the 4xx status in the 2nd example due to being in the error handler chain. I'm not certain that it does, I think it'll write a 200 status anyways.

After reading a question about the `handle_response` feature of `reverse_proxy`, I realized that we didn't have a way of serving an arbitrary file with a status code other than 200. This is an issue in situations where you want to serve a custom error page in routes that are not errors, like the aforementioned `handle_response`, where you may want to retain the status code returned by the proxy but write a response with content from a file.

This feature is super simple, basically if a status code is configured (can be a status code number, or a placeholder string) then that status will be written out before serving the file - if we write the status code first, then the stdlib won't write its own (only the first HTTP status header wins).
@francislavoie
Copy link
Member Author

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I'm convinced this could be useful. Thanks!

@mholt mholt merged commit 3f6283b into caddyserver:master Apr 8, 2021
@mholt mholt removed the under review 🧐 Review is pending before merging label Apr 8, 2021
@francislavoie francislavoie deleted the file-server-status branch April 8, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants