redirect *from* trailing slash

[chadwhitacre]

Right now we redirect to trailing slash, so if you hit /foo/bar and bar is a directory, then we redirect to /foo/bar/. I propose that we switch that behavior around, so that /foo/bar/ always redirects to /foo/bar.

[pjz]

So the rule is simple and up front: if it ends in a trailing slash, strip that and redirect to the non-trailing slash version? Then hitting /foo/bar/ will redirect to /foo/bar which could be /foo/bar.spt or /foo/bar/index.spt or whatever. What about conflicts? what if there's both a /foo/bar.spt and a /foo/bar/index.spt - which gets /foo/bar dispatched to it?

[chadwhitacre]

So the rule is simple and up front:

Yes. I would see this as another small algorithm function (redirect_from_trailing_slash), pretty early in the cycle.

What about conflicts? what if there's both a /foo/bar.spt and a /foo/bar/index.spt - which gets /foo/bar dispatched to it?

Most specific wins ... so, /foo/bar/index.spt, right? What do we do now?

[chadwhitacre]

Iirc, this affects how browsers interpret href="bar". If there's a trailing slash then it will be /foo/bar otherwise /bar. Do we care about that?

[pjz]

We care - what's the common case? If you're writing an indexfile (index.html or the like) then it's convenient to be able to refer to files in the same directory without having to put the directory name in them. If we make this change, then the writer of /foo/index.html is going to have to refer to foo/bar in their output instead of just bar. Note that we also only redirect if it's a directory, not if it's an extensionless sptfile match.

[chadwhitacre]

Discussing on March call. The question here is raised by the new (with the move to the .spt extension) ability to have both bar/ and bar.spt. Here are the combinations:

           /bar       /bar/       /bar.html       /bar.html/
bar.spt    200         302?         200             404

bar.spt    200                      200             404
bar/                   404*

bar.spt    200                      200             404
bar/index              200                     

bar/       302         404*         404             404

bar/index  302         200          404             404


* Unless autoindexing is on.

[chadwhitacre]

We trailed off with this discussion. Revisit again later.

[chadwhitacre]

We should have a master chart like the above for the whole of our dispatch, tied to test cases.

[chadwhitacre]

@pjz Link the new spreadsheet here, ya?

[pjz]

Ah, right, the WIP google-doc spreadsheet is here. Once more final, it should be turned into a document in the repo so tests can be run to make sure the code conforms to the spec.

[pjz]

I moved the google doc data into a testtable branch, along with some preliminary code that parses it out of the reStructuredText table it's in now. That format was chosen for ease of inclusion into docs - it could (should?) be pulled into a docstring for future consumption by Sphinx.

[pjz]

Okay, so there's now a test that runs and checks that the table is correct ; the output of the latest branch is at https://travis-ci.org/gittip/aspen-python/jobs/27130417 ; to understand an error, read it and interpret it in the context of the 'files' variable above.

Now that there's context, I'm coming to the conclusion that .spt is a pain. :P If the rule is 'most specific wins', then file foo should win over file foo.spt, right? and yet foo.spt should win over directory foo/ ?

[chadwhitacre]

@pjz Woo-hoo! Awesome! :D

Is dispatch w/ .spt a big enough pain to reconsider?

[pjz]

Okay, so, to explain as much to me as to you:

Currently we iterate through the segments of a request path from left to right, descending on exact match or saving variables for wildcards or whatever. So: the decision at each level of what file/directory to use is made independent of other levels... almost. There's an exception that we remember if we've encountered a wildcard-file along the way so that [ %foo.spt , /bar/baz ] + /bar/blah can backtrack to the %foo.spt since /bar/blah didn't match, but other than that the decision is level independent. The new requirement breaks that somewhat because we want [ bar.spt, bar/index ] + /bar => bar.spt ... though I guess it's only in the terminal case of our input... hmm...

[pjz]

Well the previous message helped some, so here's another:

Since we added .spt, we now have a bit of a conundrum in how to handle trying to match foo.spt and foo.html.spt. Previously, we relied on the 'direct match' of a file extension to short-circuit the finding of a negotiated simplate. This is no longer sufficient because the possibilites are now: foo.html.spt, foo.html, foo.spt instead of formerly just foo.html, foo.

[chadwhitacre]

IRC

[chadwhitacre]

Action item here is to add this to aspen.utils:

def redirect_from_trailing_slash(request):
    path = request.line.uri.path.raw
    if path.endswith('/'):
        request.redirect(path[:-1]) # + qs

With a docstring for how to use it: algorithm.insert_after('dispatch_request_to_filesystem', redirect_from_trailing_slash), and then use foo.spt instead of foo/index.spt for indices.

[pjz]

And note that foo.spt must exist, or else you'll get a nasty redirect loop.

[chadwhitacre]

Possibly we can inspect dispatch_result inside redirect_from_trailing_slash to avoid the redirect loop and give a 404 instead.

[chadwhitacre]

The result I want is to end up on /foo with a 404.

[chadwhitacre]

I.e., I don't want to be on /foo/ with a 404.

[chadwhitacre]

Another idea: subclass Response and then raise RedirectToTrailingSlash (cf. #304), catch that in redirect_from_trailing_slash.

[chadwhitacre]

How can we use the dispatch table to drive this?

[chadwhitacre]

January 2015 call

[chadwhitacre]

Something like ... ?

def redirect_from_trailing_slash(request, exception=None):
    if exception is not None and not isinstance(exception, RedirectToTrailingSlash):
        return
    path = request.line.uri.path.raw
    if path.endswith('/'):
        request.redirect(path[:-1]) # + qs

[pjz]

I think the solution is, as you mentioned, to subclass Response into a RedirectFromTrailingSlash class that gets raised on line 348 of the dispatcher. Then catch it in redirect_from_trailing_slash and turn it into a 404 (since it only happens if /foo is requested but there's no /foo.spt but is a /foo/). So more like:

def redirect_from_trailing_slash(request, exception=None):
    if exception is not None:
        if isinstance(exception, RedirectToTrailingSlash):
            raise FileNotFound
        return
    path = request.line.uri.path.raw
    if path.endswith('/'):
        request.redirect(path[:-1]) # + qs

[pjz]

Whatever the solution, it can now be pretty easily implemented in the algorithm.