a "3" shows up as the content of the page. I turned off my custom 403 page thinking that would solve the problem, but it didn't.

here's a tiny patch that makes drupal_access_denied a bit more robust (but doesn't fix the i18n problem)

CommentFileSizeAuthor
#11 05.403-404-broken_1.patch1.09 KBnickl
#10 05.403-404-broken_0.patch1.16 KBWesley Tanaka
#8 05.403-404-broken.patch1.16 KBWesley Tanaka
forbidden.patch695 bytesWesley Tanaka

Comments

Wesley Tanaka’s picture

Wesley Tanaka commented
Title: i18n module breaks access denied pages » make drupal_access_denied a bit more robust
killes@www.drop.org’s picture

killes@www.drop.org commented

What problem are you trying to fix? Where do you get a page content of "3"?

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

please explain how to reproduce

Wesley Tanaka’s picture

Wesley Tanaka commented

The 3 problem is a bug in the i18n module, not a bug in core.

This patch just makes drupal_access_denied a bit more tolerant to the case where drupal_get_normal_path is not a no-op. If you examine the patch, I think you'll agree that this is how the logic was meant to be.

Wesley Tanaka’s picture

Wesley Tanaka commented

drupal_get_normal_path actually returning something different than what is passed in being the normal case when i18n is installed.

Wesley Tanaka’s picture

Wesley Tanaka commented
Status: Needs work » Needs review
dries’s picture

dries commented
Status: Needs review » Closed (won't fix)

I don't think this needs fixing. This is merely a side-effect of another bug.

Wesley Tanaka’s picture

Wesley Tanaka commented
Title: make drupal_access_denied a bit more robust » if drupal_get_normal_path does anything at all to an empty path, drupal_not_found and drupal_access_denied break
Status: Closed (won't fix) » Needs review
StatusFileSize
new 05.403-404-broken.patch1.16 KB

Let's rhetorically completely ignore i18n for the moment.

Given that i18n is not installed, does it become an invariant of all drupal installations that (!drupal_get_normal_path('')) is always true?

If so, this patch certainly doesn't hurt anything. If variable_get('site_404', '') returns '', then given the invariant assumed above, drupal_get_normal_path('') will always evaluate the same as '' in the if ($path) guard.

If *not*, this patch fixes a bug where, if no custom site 404 or 403 page is set, variable_get('site_404','') returns '', but drupal_get_normal_path('') may in some instances return a value that evaluates as true. Given that no 404 page is set, we don't want to run a custom handler, no matter what drupal_get_normal_path returned. But without this patch, a custom handler (namely the one corresponding to drupal_get_normal_path('')) will get run, which is not the correct behavior.

The problem is in both access denied and not found functions, and the patch attached to this comment (obsoleting the previous patch) addresses both.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Your patch - comment #8 - worked well for me. I was trying to followup with the other bug reports that were displaying:

====
Primary links
2
====

But they weren't working for me. Realized it was only on links that were 404'd, found this patch and applied it. Presto!

Thanks. Worked on Drupal's cvs code & i18n (although that might be irrelevant).

Mike

Wesley Tanaka’s picture

Wesley Tanaka commented
StatusFileSize
new 05.403-404-broken_0.patch1.16 KB

patch again applies to HEAD

nickl’s picture

nickl commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 05.403-404-broken_1.patch1.09 KB

Rerolled to head.
Dries commented in #7 that this won't be fixed but maybe wtanaka has a point in #8.
Either way, simple patch, makes logical sense please commit or close this issue.
Tested works as advertised.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Closed (won't fix)

There are 2 ways for drupal_get_normal_path() to turn '' into something:
1. Putting a URL alias on '', which the validation on that form doesn't allow.
2. Through implementing custom_url_rewrite(), which i18n must be using.

#1 is ruled out due to impossibility and #2 is a known bug in i18n. What is left to fix in core?