Proxito: don't check for index.html if the path already ends with /.
#10153
Conversation
| tryfiles = ["index.html", "README.html"] | ||
| # If the path ends with `/`, we already tried to serve | ||
| # the `/index.html` file, so we only need to test for | ||
| # the `/README.html` file. | ||
| if full_path.endswith("/"): | ||
| tryfiles = ["README.html"] |
There was a problem hiding this comment.
This is really good! We are reducing 1 call to S3 for all the 404 pages 💪🏼
| def test_redirects_to_correct_index(self, storage_exists): | ||
| """This case is when the project uses a README.html as index.""" | ||
| def test_redirects_to_correct_index_ending_with_slash(self, storage_exists): | ||
| """When the path ends with a slash, we try README.html as index.""" |
There was a problem hiding this comment.
Eventually, we should remove this and stop trying README.html. That's a pretty legacy decision that I don't think it applies anymore.
We should probably start with new projects and keeping this behavior only for old projects and remove this logic finally after a couple of months.
There was a problem hiding this comment.
I'm in favor of removing it or documenting it explicitly. This is not expected default functionality from a webserver/host although it isn't a super uncommon reconfiguration.
There was a problem hiding this comment.
Yea, we should start by logging when this happens, so we know how many people it will effect.
There was a problem hiding this comment.
@ericholscher we are already logging this 😉 . Take a look at this https://onenr.io/0LREdxdX5wa
There are more project than I've expected doing this 😞
There was a problem hiding this comment.
We have 340 projects in .org and 1 in .com https://onenr.io/0BQ1Gv2BMRx
There was a problem hiding this comment.
we should continue this at #9993
If the path ends with
/, we already tried to serve the/index.htmlfile and weren't able to find it, so we only need to test for the README.html in those cases, for paths that don't end with slash, we need to check for both files.