Let custom_url_rewrite_outbound() operate on base_url
c960657 - January 7, 2008 - 17:43
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | path.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
The subject of http://drupal.org/node/150049 was "Let custom_url_rewrite operate on querystring and fragment". I suggest letting custom_url_rewrite_outbound() work on base_url as well. This will allow users to override the hostname, port and base_path. This may be useful e.g. when Drupal is running on a non-standard port behind a reverse proxy or in certain load-balancing setups.
I have attached a very simple fix for this.
| Attachment | Size |
|---|---|
| common.patch | 1.04 KB |
#1
#2
Please don't set your own patches ready to be committed. All patches need community review, and *especially* when they're touching a function that's used basically everywhere.
Could you clarify what we need to do to test this?
#3
Sorry, I chose the "ready to be committed" by mistake.
For testing purposes, you can add the following function to settings.php. It directs all requests to /admin to www1.yoursite.example.com, while the rest is served by www2.yoursite.example.com. Both hostnames should point to the same server, and the site should reside in sites/yoursite.example.com.
$cookie_domain = 'yoursite.example.com';function custom_url_rewrite_outbound(&$path, &$options, $original_path) {
global $user;
$prefix = (strpos($path, 'admin/') === 0) ? 'www1' : 'www2';
// Strip www1/www2 if present and insert $prefix
$base_url = preg_replace('@^http://(www1.|www2.)?([^/:]+)?@', 'http://' . $prefix .'.\2', $options['base_url']);
if ($base_url != $options['base_url']) {
$options['base_url'] = $base_url;
$options['absolute'] = true;
}
}
#4
this would be very useful for serving static content from a different domain like files.example.com, for example.
#5
I have use-cases for this as well, over in the Domain Access module.
In some cases, I need to force $absolute values to be attached to URLs -- for example, when searching content from multiple domains, the links need to go to sites where access rules allow the nodes to be viewed.
In 5.x, I had provided a hook_url_alter() patch to provide this functionality, and have tried to backport custom_url_rewrite_outbound() since learning of the function.
http://drupal.org/node/210248
However, custom_url_rewrite_outbound() fires too late in the url() function for me to make any modification to the $options['absolute'] parameter.
This patch addresses that need, however, I think it introduces the potential to leave $base unset if the IF check for (!empty($path) && $path != '') returns FALSE.
Perhaps we should move the custom_url_rewrite_outbound() sequence instead?
For reference, my approach was to run the rewrite hook at the very top of the url() function.
http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/domain/patc...
#6
I don't see which code path leads to $base being unset? That is, not unless the custom_url_rewrite_outbound() function calls unset($options['base_url']) or similar, but that would be bad behaviour by the custom_url_rewrite_outbound(), wouldn't it?
However, it probably is a good idea to move the custom_url_rewrite_outbound(). The current implementation doesn't support rewriting URLs to the site root (i.e. when $path == ''). This updated patch fixes this.
BTW, see also a related issue regarding rewriting URLs to uploaded files: http://drupal.org/node/207310
#7
With regard to $base, I may have read the code incorrectly.
I like this new patch, since it is exactly what I was going to propose.
#8
The patch works, and you can rewrite $options['base_url'] and set $options['absolute'] = TRUE in order to force links to go to different hosts. It also preserves url aliases.
There are, of course, performance implications, but that is a trade-off for added functionality for sites that choose to implement the function. I have not done extensive performance testing.
Marking RTBC because I'd like to get input from core committers.
#9
This is an API change for the custom rewriter too late for 6.x.
#10
FWIW, with this patch applied the API still adheres to the documentation at http://api.drupal.org/api/function/custom_url_rewrite_outbound/6.
The documentation does not indicate that only certain values in $options can be changed, so one could argue that this is not an API change but a bugfix.
#11
The argument is that this is a bugfix, since the original feature did not consider all use cases.
I will reset this issue once. If it is rejected as too late in the release cycle, then I accept that it is a feature for 7.x.
If it does not go in to 6.x, I will be releasing this patch (or something like it) as part of the Domain Access module.
#12
Well, the patch is also an API change in that it removes the possibility to modify $options['prefix'], although it is supposed to be modifiable from the function. Let's try to not break APIs when you are advocating a patch as a bug fix. :)
#13
Sorry, that was not intended. Here is another try that retains the ability to alter $options['prefix'].
I reversed the if statement, now that the else part is only relevant when $path == '<front>'.
I assume that the purpose of
trim($options['prefix'], '/')is to remove the trailing slash, so I changed it to rtrim() to make this clearer. I hope this makes sense.#14
works perfectly. the only thing i did not test is interaction with locale. hopefully gabor can verify that.
here is the function i used to test (note base url change at bottom). i will change the docs accordingly once this goes in.
<?php
function custom_url_rewrite_outbound(&$path, &$options, $original_path) {
global $user;
// Change all 'node' to 'article'.
if (preg_match('|^node(/.*)|', $path, $matches)) {
$path = 'article'. $matches[1];
}
// Create a path called 'e' which lands the user on her profile edit page.
if ($path == 'user/'. $user->uid .'/edit') {
$path = 'e';
}
$options['base_url'] = 'http://cnn.com';
$options['absolute'] = true;
}
?>
#15
Well, this patch also fixes URL encoding of the path prefix when we are on the front page, which makes it all consistent with the path handler case, so committed, thanks.
#16
Attached is a backport for Drupal 5, if anyone needs it.
#17
Automatically closed -- issue fixed for two weeks with no activity.