drupal_access_denied handler should know what page was requested
moshe weitzman - November 29, 2004 - 02:37
| Project: | Drupal |
| Version: | x.y.z |
| Component: | menu system |
| Category: | feature request |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed |
Description
the ability to specify a path for handling 'access denied' pages is nice. it would be helpful if this handler could learn the path which was originally requested by the user. currently, $_GET['q'] is overwritten with the path of the handler and thus the handler is ignorant.
#1
for now, i work around this limitation by stashing the path in my own var using the following line of code which executes in hook_menu(!$may_cache)
$_GET['q_original'] = $_GET['q'];
#2
This same situation occurs when a node is a member of a book. Via the _nodeapi hook in the book module eventually the function menu_set_active_item() will execute the following statement:
<?php
if (empty($path)) {
$path = $_GET['q'];
}
else {
$_GET['q'] = $path;
}
?>
... which sets the superglobal $_GET to the callback path of the menu item.
Like Moshe, I had to store the original contents in another variable. I need to use the full value of the $path in a module that extends the node to accept "full directory paths" and this works fine when the node is just a node, it breaks only when it is part of a book.
I guess what I am asking is "can we find another approach to menu/callback $path setting. Perhaps declare a variable for menu in the bootstrap and then leave the $_GET alone. It would be much more intuitive for module/theme developers to depend on the content of $_GET as reflecting what they see in the URL.
#3
I'm changing this to CVS because:
- I think it's worth looking into further
but
- any solution will not likely be back-ported.
#4
Worth noting that Kbahey has already posted a patch for common.inc that maybe useful in this situation.
http://drupal.org/node/35687
The patch was intended for his customerrror.module, but, I'm guessing that it maybe aapplied to common.inc across the board.
Dub
#5
Here is the patch to common.inc for HEAD.
It is one line, and just stores the destination in the session. Any other module or function can check if destination is set and act accordingly.
For example, customerror traps the user hook, checks if it is a login, unsets the destination in the session, and redirects to the original page the user tried to access
If we get this in, modules can act on this in many ways.
#6
session is for data that needs to persist across page views. $_GET[q] doesn't need this. i don't really like this solution.
#7
#8
I would say "won't fix" 'cause why would you preserve the original value of $_GET['q']? I believe the core doesn't need this and if it is need by a contrib module, the original value could be capture from the init or menu hooks.
Someone that could benefit from this approach would be the statistics module, when it saves the path in the log, instead of $_GET['q'], it could use $original_path. Say $original_path was saved in the menu hook like this:
<?php...
if ($may_cache) {
...
}
else {
global $original_path;
$original_path = $_GET['q'];
...
}
...
?>
In statistics_exit, after this:
<?phpdrupal_bootstrap(DRUPAL_BOOTSTRAP_PATH);
?>
it could do this:
<?phpglobal $original_path;
if (!isset($original_path)) {
$original_path = $_GET['q'];
}
?>
And use $original_path instead of $_GET['q'] in the INSERT statement.
It works. ;-)
#9
the use case is to put a login form on the accedss denied page ()assuming anonymous user). that login form ought to know the destination where the user was headed.
#10
so you could save the original init path by drupal core itself and provide a normalized method to get its value. Say drupal_init_path() is modified like this:
<?phpfunction drupal_init_path() {
global $drupal_init_path;
if (!empty($_GET['q'])) {
$_GET['q'] = drupal_get_normal_path(trim($_GET['q'], '/'));
}
else {
$_GET['q'] = drupal_get_normal_path(variable_get('site_frontpage', 'node'));
}
$drupal_init_path = $_GET['q'];
}
?>
The normalized method to get the original path could like:
<?php/**
* Get the original init path.
*/
function drupal_get_init_path() {
global $drupal_init_path;
return $drupal_init_path;
}
?>
So you could set destination based on drupal_get_init_path() for a login form (or anything else) defined inside the 403/404 handlers.
#11
Then the example I posted above, related to the statistics module, would be much simpler. The exit hook could use drupal_get_init_path() rather than $_GET['q'] to save the path in the access_log table.
#12
yes, thats the right idea. perhaps we change to two functions get_initial_path and set_initial_path. the set_initial_path will be called by the menu system whenever it first figures out $_GET['q'] ... lets not abbreviate init instead of initial. dries no like that.
#13
forget my last comment. i didn't realized we already had that init_path function. markus' proposal looks good to me.
#14
Do you mean this? (see patch)
#15
rerolled (affected line in statistics module was changed)
#16
This is so simple that I believe it is RTC. Please, correct me if I'm wrong.
#17
I suspect that http://drupal.org/node/13621 is a bug that depends on the patch here
#18
I meant http://drupal.org/node/56142
#19
using a get_foo function with a global variable doesn't make much sense. There should be a set_foo function and a static var.
#20
This issue offers a much simpler solution to this (and another) issue:
http://drupal.org/node/55869
#21
tentatively marking this as fixed based on the patch that steven points to. please reopen with some detail if that solution isn't workable. it persists the original path in the $_REQUEST['destination'] element
#22