We have moved to GitHub Issues
Created by Jeavon Leopold 26 Jul 2013, 10:28:44 Updated by Sebastiaan Janssen 05 Jun 2015, 16:28:23
Is duplicated by: U4-2620
Custom 404 does not work for a node that does not have a template instead you get the "intentionally ugly" message
Looks like none of the 404 handlers are being hit when the page has no template.
Er... unintended consequence of other things, I think. When a page has no template we handle the request over to MVC, just in case some route hijacking is in place. If no route hijacking exists then we go through the "last chance" content finder... and a few versions ago we moved the 404 handlers to the "normal" content finders phase.
So it might be the case that they don't run anymore when there's no template. Will check asap and fix if required.
OK, here's how it goes: the pipeline has two cycles of finders. Cycle 1 runs the IContentFinder implementations until one of them returns true (and a document to display). Cycle 2 runs the unique "last chance" IContentFinder if cycle 1 returned no document, or a document with no template.
There's a special IContentFinder that's been created for backward compatibility and runs all the legacy NotFoundHandlers. In the past it ran as the "last chance" finder but at one point in time it was moved to cycle 1 -- that makes more sense.
However that also means we don't have a "last chance" finder anymore and that hurts when there's no template. So I need to restore a "last chance" finder but really that one should ''only'' run the true ''last chance'' NotFoundHandler (the one that handles true 404) and not all the other handlers.
Right now I don't know how to do that. Best solution I can see is to create a special IContentFinder to be used as the "last chance" finder, that would run ''only'' the ''last'' NotFoundHandler of the list, assuming it's the 404 one.
Should work in most cases and buy us tranquility until we remove support for NotFoundHandler entirely in v7. Should also be rather easy to code.
More: running only the last of the NotFoundHandlers is going to be a bit obscure. So probably we'd want to run all of them, but with a special mode that requires a template. Looking into it.
Happy to take your lead on this one mate, whatever you think is best.
Might not be such a good idea to run then all ''again'' after all. So I'd really would like to have a way to differenciate between the "normal" NotFoundHandler and the "last chance" NotFoundHandler ie those that are supposed to manage a true 404 situation.
And I guess there's only one per setup.
Would it be acceptable to add a config attribute to tag those handlers that are "last" eg:
It goes beyond the "missing template" issue. If an internal redirect points to a missing page, or if a document is protected by a login page which happens to be missing... then it's the last chance handler that takes care of those situations. And we really don't want to run them all again in those cases.
I don't have the deep insight into the source code, but for what it's worth, I'm not much of fan of changing the config files. In upgrade scenarios this often prove error prone. It should be a last resort.
I hope we can figure out a good solution for this case.
Yeah, the config change feels far from ideal to me too.
What's the problem with running all of the handlers again (apart from a slight performance hit)?
This is exactly what I would expect to happen: no template is found, so we go into 404 mode, then the NotFoundHandlers kick in one by one until we have a match.
As an end-user, I don't care too much that they are ran multiple times, just as long as I get the expected result.
Also performance when handling 404 errors/exceptions isn't so big of a concern. Of course it shouldn't take down the server, but a even an extra 50 milliseconds shouldn't be a problem.
@Sebastiaan: "''NotFoundHandlers kick in one by one until we have a match''"... and that match has a template. Else we're going to get the same first document that had no template. But I think that should be easy to check because at that point (404) we don't want to handle alternate templates at all.
Now the "last chance" finder is also supposed to take care of the following scenario: a content finder finds a document that has a template... but is protected (visitor has no access). So member is redirected to the login page... but that page does not exist (or has no template). Can't be displayed. In that case we call the "last chance" finder to route back to the 404 page.
With the proposed solution (run all handlers again) we would find the protected document ''again'' and so we'd enter a loop. My mind being twisted, there's been protection against such infinite loops in Umbraco since 4.10 I think... so you'd get an exception instead of the 404 page.
Would that be acceptable? I think yes... then I can try to code the finder tonight.
@Stephan Yeah, I get the subtleties now...
Well, first of all, we don't have time to implement AND test this properly for 6.1.4 so no rush.
I don't mind an exception being thrown if there's going to be an infinite loop. Just to be clear: we wouldn't need to chance config files with the proposed change, right?
I ended up thinking about this silly issue at 4am! Imagine the following situation:
A request for "/about-us/our-values/foo" should end up 404 because there's a document at that url, with no valid template. However if we run the whole set of handlers again, here's what happens:
So instead of 404 we end up displaying document "/about-us/our-values" using template "foo" which is totally unexpected. Granted, this is a corner case, but that's precisely because of these silly cases that the "last chance" finder notion was introduced.
So... don't think I'll make it for 6.1.4, and still trying to figure out something. The point is, I think I introduced the "last chance" finder precisely because a single set of finders can't do it... in other words I'm a bit afraid that whatever I do, there'll be a buggy corner case.
@Stephan There was once a long discussion about letting altTemplate= apply any template to a page. The suggestion was to make the user select the template(s) that can be used to display a page and the altTemplate would also honour those selections.
For example, the 'RSS' template would need to be allowed on the News and Blog doctypes in addition to the regular templates associated with those doctypes. Its a bit more effort for the site builder but not a lot, even for an upgraded site.
Originally, the argument for this centered around the desire to keep a site more secure but I don't think it was a compelling argument and as far as I know was never implemented.
My question is... would your not-found handlers benefit from such a requirement? Then pages that don't exist, but are aliases for a template that is allowed for the page would work and all others would know they were 404s.
In your example above, /about-us/our-values/foo (which is a page with no template and 'foo' isn't a template alias allowed on the our-values page) would know it is a 404 and display the 404 page.
It could also simplify these cases, would it?
If you also had this situation in the above:
My point here is to raise the question about requiring all templates that can display a page to be specified, rather than using the current situation that is both ambiguous and potentially a security concern for some. If it were to resolve the last issues with 404 handlers and some routing quagmires that would be reason enough to implement it IMO.
@Doug: from a pipeline point of view, restricting the templates that are valid for altTemplate on a give document type (or even document) should be rather easy because the whole altTemplate management is centralized in one place. What we'd need is a proper UI, though. Suggesting you create another issue / feature request.
As for the current issue... it would help solve ''part'' of it but not the whole -- which is solved by having a special finder handle the true "not found" situation (ie, I can't display what you're asking for).
Unfortunately the NotFoundHandlers (404handler.config) does not support indicating which handler would be the true "not found" one (vs. those handling url aliases, member profiles...). I'm trying hard to figure out how to ensure proper 404 management without modifying the config file... but haven't been able to find a solution yet.
Actually doubting there's one, as that concept of a special handler to handle "true" not found situation was introduced precisely as the solution. Now, we plan to deprecate NotFoundHandler in v7 in favor of the new ContentFinder. Dunno what to do in the meantime.
Mean, if it either modify the config file, or live with some buggy corner cases... what would be easier for ppl to understand?
@Stephan - a non-ui workaround for now would be to require that the existing "allowed templates" checkbox list be consulted for any altTemplate use as well. Not quite as nice as having a separate list for altTemplates in the UI but it could be implemented immediately which has an advantage.
Of course it doesn't handle the whole situation... heck, I don't even UNDERSTAND the whole situation and am delighted (and marvel) that you do and are doing such a good and thorough job with it!
IMO, a few buggy edge cases are, assuming they really are edge cases. Trouble is, what is an edge case to one person is common practice to another. Especially with upgraded sites that used to work but might not after an upgrade. If a config file change could resolve the problem of edge cases that would be a better solution. Upgrades are important and documentation (and possibly even an upgrade wizard?) will go a loooong way to avoiding broken sites after an upgrade.
Yep. Spent a bit of time today thinking of it... thinking that a properly documented addition to a config file is cleaner that a list of complex non-working edge cases. Trying to code it tomorrow.
OK, have coded the new config attribute. So you add it to you "last chance" handler, ie the one that handles the true 404 cases, and it all should work properly, in every cases (missing template, missing login page...):
I find it much easier to explain, to document & to support compared to all the other black magic attempts I've made. Feel free to test & comment, it's in 6.2.0, commit 3fc4f3685950d3436f58b081cfa1cbf7e878b878.
I just had a thought. Couldn't you try to "guess" which one handles the true 404 cases? I guess that in most cases it would be assembly="umbraco" type="handle404". Then if you want to override that, you good make use if the last=true attribute?
That would allow most installations to be upgraded without making any config changes.
Would be quite easy to always register "handle404" as a "last chance" handler. Sounds a bit like "dark magic" to me but why not. What do others think? Sebastiaan? Re-assign to me if you want me to do it.
Crazy idea from the "convention over configuration" department... what if the last entry in the
Sometimes "dark magic" is the best solution, as it "just works" :-) But perhaps Doug's suggestions is the better convention, the last entry is the last resort before the IIS takes over. And since some people might have setup
then you will have a blank 200 response from the server, if there is no action on any not found handlers.
Vote for Doug :)
Great feedback, great discussion, thanks!
OK, I guess in 99.99% cases ppl will have their "last chance" handler as the last entry. And in 90% cases that will be handle404. So if that's OK with everybody, I'll move to the following logic: by convention, '''the last handler in the list is used as the "last chance" handler.'''
That is, '''as long as the registered "last chance" content finder is the special one that redirects to the handlers'''. If it's not anymore, we assume you've setup your own "last chance" content finder (or make sure there's none) and therefore no handler is the list is the "last chance" one.
Sounds good? Reassigning to myself.
Go for it :-)
I"m liking it! Easier coding for Stephan and no significant changes to the config file. Nice.
Only thing I don't understand, @Stephan, is how someone would ensure there is NO "last chance" handler if the last one in the list is always assumed to be the "last chance" handler by convention. Obviously people can replace the default handle404 with their own but how would someone have no last chance handler at all? For that matter, why would anyone ever want such a situation since it would break so many things?
@Doug: we're not using the handlers directly anymore. The "new thing" is content ''finders'' and on the long term we'd like people to write finders, not handlers. To ensure backward compatibility, we still indirectly support handlers through two special finders:
By default, the last chance ''finder'' is configured to be ContentLastChanceFinderByNotFoundHandlers. But you can replace it by whatever finder you want (your own implementation of a 404 finder). Or you can reset it to indicate that you have ''no'' last chance ''finder''.
In both cases, I think we can assume that since you do not seem to want to pick the last chance handler in the config list, then that list contains ''no'' last chance handler, and therefore they should all run as normal handlers. Am I making sense?
Now, about having ''no'' last chance handler or finder... well in that case you just fall back to the "ugly" Umbraco 404 page. Your choice. Does not make much sense, but... as IT grand master Liu Tchu liked to say, "the less sense it makes, the more probable it is that some user will try it".
And also if you begin writing your own finders and handlers, you properly know what you are doing. Now we are fixing a bug that hits people going from a pre 4.11 - without any config file changes. Nice :-)
@Stephan - I can't keep up! O.o Thanks for the explanations, it helps a lot. As @Emil says, going forward there is a better solution and for v4 and v6 this is also a good solution. So wins all around. Well done!
Done, commit 0ed4c1868be531ca6c9806a9fad54210b2d00120 in 6.2.0.
Now the rules are:
Would be good to get all this documented: http://our.umbraco.org/documentation/using-umbraco/config-files/404handlers/
hint hint ;)
Backwards Compatible: True
Affected versions: 6.1.1, 6.0.7
Due in version: 6.1.5