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

Allow to set response headers on error pages #1279

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pkamps
Copy link
Contributor

@pkamps pkamps commented Feb 15, 2017

In ezp it's not possible to set custom response headers for error pages. ezp only allows you to set response headers by requested URLs (see HTTPHeaderSettings in site.ini).

This pull request allows you to specify custom response header for error pages on ezp. That's an important feature in case your site is behind a reverse proxy (CDN akamai, CloudFront, Varnish) and you control the caching TTLs with response headers.

You can test this pull request by accessing a URL that does not exists. From that request you should get a response with this header: 'Cache-Control:public, must-revalidate, max-age=300'

The only small complexity in this pull request is the fact that we re-use the 'Status' header in order to build the response status code - for example '404 Not Found'.

@gggeek
Copy link
Contributor

gggeek commented Feb 15, 2017

bc break, should at least be documented as such

@pkamps
Copy link
Contributor Author

pkamps commented Feb 15, 2017

You're right. I probably can extend the pull request and still support the old settings variable 'HTTPName'. But is it worth it? In all my ezp projects I never had to change the HTTP response status message for error pages. I don't even know why that's configurable - maybe to translate it into another language?

@pkamps
Copy link
Contributor Author

pkamps commented Jun 23, 2017

Latest commit is avoiding the BC break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants