when viewing page node/123 , if this node doesn't exist, the front page is displayed instead of 404.

CommentFileSizeAuthor
#13 82764.patch577 bytesWesley Tanaka
#7 node-not-found.2.diff984 bytesbeginner
#4 node-not-found.diff1.06 KBbeginner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

doq’s picture

doq CreditAttribution: doq commented
Version: x.y.z » 4.7.3
beginner’s picture

beginner CreditAttribution: beginner commented
Version: 4.7.3 » x.y.z

no: 4.7.3 seems to work ok:
http://drupal.org/node/827645
(i.e. you get 'page not found').

doq’s picture

doq CreditAttribution: doq commented
Title: when viewing page node/123 , if this node doesn't exist, the front page is displayed instead of 404 » if go to node/123 or node/blabla & if page doesn't exist - 'node' page displayed instead of 404
Version: x.y.z » 4.7.3

try http://demo.opensourcecms.com/drupal/node/blablabla

beginner’s picture

beginner CreditAttribution: beginner commented
Title: if go to node/123 or node/blabla & if page doesn't exist - 'node' page displayed instead of 404 » if node/nnn doesn't exist, return 404 instead of front page.
Version: 4.7.3 » 5.x-dev
Status: Active » Needs review
FileSize
node-not-found.diff1.06 KB

This page handles node/$nid where $nid does not exist.
Does a callback already exist that simply returns drupal_not_found()?

I am not handling the case node/blahblah.
If you feel it should be handled, add a check on arg(1) at the beginning of the node_page_default() function, or add another callback in node_menu().

Patch against HEAD.

beginner’s picture

beginner CreditAttribution: beginner commented

A very similar problem exists with user/nnn.
Since the solution offered is very similar, it is worth considering merging the two patches into one: http://drupal.org/node/108579

RobRoy’s picture

RobRoy CreditAttribution: RobRoy commented
Status: Needs review » Needs work

Couldn't we just make the menu callback 'drupal_not_found' since it now does the printing of the error, the footer then exits? Instead of having a wrapper callback?

beginner’s picture

beginner CreditAttribution: beginner commented
Status: Needs work » Needs review
FileSize
node-not-found.2.diff984 bytes

Very good. How silly of me!

joshk’s picture

joshk CreditAttribution: joshk commented
Status: Needs review » Reviewed & tested by the community

Patch applies clean, works as advertised and seems "right" to me.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Closed (duplicate)

http://drupal.org/node/108695

beginner’s picture

beginner CreditAttribution: beginner commented
Status: Closed (duplicate) » Needs review

I just updated to HEAD, and I find the symptoms to be the same as before.
The patch is still needed.

Wesley Tanaka’s picture

Wesley Tanaka CreditAttribution: Wesley Tanaka commented

also dupe: http://drupal.org/node/90780
?

Wesley Tanaka’s picture

Wesley Tanaka CreditAttribution: Wesley Tanaka commented

It seems to me like you could remove 'title' => t('View'), since that will get set by drupal_not_found(), and "View" doesn't make sense for a not found page anyway.

Wesley Tanaka’s picture

Wesley Tanaka CreditAttribution: Wesley Tanaka commented
FileSize
82764.patch577 bytes

I see that t('View')'s original pedigree was probably the View/Edit/Track/etc tabs. So I'm more confident that it's not useful here. New patch attached which is the same as http://drupal.org/files/issues/node-not-found.2.diff, but does not set 'title'

Wesley Tanaka’s picture

Wesley Tanaka CreditAttribution: Wesley Tanaka commented
Status: Needs review » Needs work

actually, this patch only works if the thing following node/ is a number.

so node/cookie still gives the default node page with this patch, even if that path doesn't exist.

beginner’s picture

beginner CreditAttribution: beginner commented
Status: Needs work » Reviewed & tested by the community

Actually, I already said in #4 that the patch only handles cases where we have an integer after node/...
This is by far the most common case, and the easiest to handle without adding too much cruft.
How often do you have an alias "node/whatever"? But people do post links to nodes node/1234 which subsequently disappear (are deleted).

The patch is simple and solves a very common problem.

Wesley Tanaka’s picture

Wesley Tanaka CreditAttribution: Wesley Tanaka commented

In my case, for some reason, googlebot got a hold of several different broken "/node/something" links (the correct path was: /something), and then it started going through the pager: /node/something?page=1, /node/something?page=2, . . . /node/something?page=983

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Needs work

This is still inside

    if (arg(0) == 'node' && is_numeric(arg(1))) {

so 'node/non-numeric-gibberish' still goes to the front page.

deanypop’s picture

deanypop CreditAttribution: deanypop commented
Priority: Normal » Critical

I'm just your average bear, but I'd like to bump the severity of this, since there is now an easily-exploitable way to get the drupal "welcome to your new site" page for any site that doesn't use the node/front page promotion combo.

In fact, this is worrisome for every site out there under 5.x, anyway, because of the failure to 404 non-existent pages.

But, you know, I can't fix it, so I'll go away now.

mr700’s picture

mr700 CreditAttribution: mr700 commented

subscribint

toemaz’s picture

toemaz CreditAttribution: toemaz commented

Having the same issue with a 5.1 installation.

Gerhard Killesreiter’s picture

Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented
Status: Needs work » Closed (duplicate)

http://drupal.org/node/90780