U4-2549 - Custom 404 not working when no template exists

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

More info:

http://our.umbraco.org/forum/developers/api-questions/41013-Custom-404-not-working-for-No-template-exists

Comments

Sebastiaan Janssen 26 Jul 2013, 13:05:09

Looks like none of the 404 handlers are being hit when the page has no template.


Stephan 26 Jul 2013, 19:51:20

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.


Stephan 27 Jul 2013, 06:03:05

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.

Comments?


Stephan 28 Jul 2013, 10:36:21

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.


Shannon Deminick 30 Jul 2013, 00:04:19

Happy to take your lead on this one mate, whatever you think is best.


Stephan 16 Aug 2013, 15:40:46

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.

Comments?


Emil Rasmussen 19 Aug 2013, 09:12:08

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.


Sebastiaan Janssen 19 Aug 2013, 09:42:51

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.


Emil Rasmussen 19 Aug 2013, 11:34:27

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.


Stephan 20 Aug 2013, 16:17:09

@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.


Sebastiaan Janssen 21 Aug 2013, 11:36:36

@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?


Stephan 21 Aug 2013, 13:12:54

I ended up thinking about this silly issue at 4am! Imagine the following situation:

  • You have a content at "/about-us/our-values" - with a valid template
  • You have a content at "/about-us/our-values/foo" - with no template
  • You have a template with alias "foo"

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:

  • find by url = finds document at "/about-us/our-values/foo"
  • so we have a document
  • but no valid template => run them all again & require a template
  • find by url = finds document again, but it has no valid template, fail
  • find by url/template = finds document at "/about-us/our-values" with valid template "foo"

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.

Stay tuned...


Douglas Robar 22 Aug 2013, 13:27:40

@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?

  • content at "about/values" - with a valid template
  • a template aliased "foo" When requesting about/values/foo it isn't a page with a valid template, and though 'foo' is a valid template alias it isn't allowed on the values page. Therefore 404. Or, if 'foo' is allowed on the values page you display the values page using the foo template.

If you also had this situation in the above:

  • content at "about/values/foo" - with no template You would display the 404 because the about/values/foo page exists but has no template. AltTemplate is only considered after looking for real pages so about/values/foo finds the page with no template rather than the about/values page with the foo template. Content nodes win over template aliases.

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.

cheers, doug.


Stephan 22 Aug 2013, 15:25:10

@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?


Douglas Robar 22 Aug 2013, 17:21:24

@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.

cheers, doug.


Stephan 27 Aug 2013, 18:15:13

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.


Stephan 28 Aug 2013, 16:21:09

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.


Emil Rasmussen 29 Aug 2013, 06:57:04

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.


Stephan 29 Aug 2013, 07:04:06

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.


Douglas Robar 29 Aug 2013, 09:15:37

Crazy idea from the "convention over configuration" department... what if the last entry in the was always used as the "last chance" handler? No need for a config file change. On upgrade, add the "handle404" at the end of the list of notfoundhandlers. Would that work?


Emil Rasmussen 29 Aug 2013, 11:29:03

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 :)


Stephan 29 Aug 2013, 12:00:12

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.


Emil Rasmussen 29 Aug 2013, 12:06:00

Go for it :-)


Douglas Robar 29 Aug 2013, 12:11:53

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?


Stephan 29 Aug 2013, 12:25:56

@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:

  • '''ContentFinderByNotFoundHandlers''' - runs the NotFoundHandlers
  • '''ContentLastChanceFinderByNotFoundHandlers''' - runs the "last chance" NotFoundHandler

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".


Emil Rasmussen 29 Aug 2013, 12:33:54

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 :-)


Douglas Robar 29 Aug 2013, 12:39:32

@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!


Stephan 29 Aug 2013, 15:34:00

Done, commit 0ed4c1868be531ca6c9806a9fad54210b2d00120 in 6.2.0.

Now the rules are:

  • if your last chance finder is ContentLastChanceFinderByNotFoundHandler (which is the default) then the last entry in handler404.config is used as the "last chance" handler.
  • otherwise, if you've changed your last chance finder via ContentLastChanceFinderResolvoer, every entry in handler404.config is used as normal handler.


Shannon Deminick 31 Aug 2013, 01:35:19

Would be good to get all this documented: http://our.umbraco.org/documentation/using-umbraco/config-files/404handlers/

hint hint ;)


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 6.1.1, 6.0.7

Due in version: 6.1.5

Sprint:

Story Points:

Cycle: