Skip to content

Redirect index files in proxito instead of serving #6387

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

Merged
merged 5 commits into from
Nov 19, 2019

Conversation

ericholscher
Copy link
Member

This lets us just return a redirect if the file exists. This matches the
existing nginx behavior, except it actually appends the index filename
instead of just a slash.

I'm not sure which is better, but this is more explicit which feels good.

This lets us just return a redirect if the file exists.
This matches the existing nginx behavior,
except it actually appends the index filename instead of just a slash.

I'm not sure which is better,
but this is more explicit which feels good.
@ericholscher ericholscher requested a review from humitos November 18, 2019 16:34
@humitos
Copy link
Member

humitos commented Nov 18, 2019

We had a discussion on Slack. The first approach had a problem that users will see the index.html in the URL when they weren't hitting an URL with a trailing slash --which would be a problem for people using dirhtml builder.

So, we decided to change this behavior to redirect to the same URL with a trailing slash if the index.html file exists. This redirect will be caught again by El Proxito and return the X-Accel header that will be include the full path but the user won't see the index.html in the URL.

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Happy to be pretty close to test El Proxito in production :)

@ericholscher ericholscher merged commit 2a83be0 into master Nov 19, 2019
@ericholscher ericholscher deleted the proxito-redirect branch November 19, 2019 09:33
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