Remove hard-coded check for "Apache" for deciding whether to add "index.php" within url() [#437228]

The url() function tries to handle the situation of a web server where index.php isn't setup as a default document and clean urls are not in use with the following code at the end of the function:

if ($query = join('&', $variables)) {
  return $base . $script . '?' . $query . $options['fragment'];
}
else {
  return $base . $options['fragment'];
}

However, this code only adds the $script variable when $variables is not empty. In the case of calling url(<'front>'), $variables is empty, and therefore, 'index.php' is not appended to the url.

Should the last line be changed to this?

return $base . $script . $options['fragment'];

Perhaps this isn't a common use case, since one would assume that a real, live website would be configured to know how to handle a request to the base url of a domain, but if one is setting up a drupal micro-site within a subfolder of a domain, then this can't be assumed.

Comment	File	Size	Author
#16	url-fix_script-437228-16.patch	2.39 KB	effulgentsia
#16
#12	url-fix_script-437228-12.patch	2.65 KB	effulgentsia
#12
#11	url-fix_script.patch	2.99 KB	effulgentsia
#11
#3	url-script-on-front-page-437228.patch	602 bytes	effulgentsia
#3
#1	url-fix_for_iis-437228-0.patch	402 bytes	effulgentsia
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

effulgentsia CreditAttribution: effulgentsia commented 8 August 2009 at 01:35

Status:

Active

» Needs review

File	Size
url-fix_for_iis-437228-0.patch	402 bytes

Patch attached.

Comment #2

26 October 2009 at 16:45

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #3

effulgentsia CreditAttribution: effulgentsia commented 28 October 2009 at 22:28

File	Size
url-script-on-front-page-437228.patch	602 bytes

Re-rolled. I'm still confused as to whether there's a reason why HEAD isn't this way (intentional or oversight), and if intentional, let's document why.

Comment #4

effulgentsia CreditAttribution: effulgentsia commented 28 October 2009 at 22:29

Status:

Needs work

» Needs review

With the status change this time.

Comment #5

mr.baileys

Dutch

🇧🇪 (Ghent)

CreditAttribution: mr.baileys commented 1 March 2010 at 08:05

Confirmed the bug on IIS 7.0 by removing index.php from the list of default documents and switching off Clean URLs. Clicking the "home" icon results in a menacing

HTTP Error 403.14 - Forbidden
The Web server is configured to not list the contents of this directory.

Applying the patch fixes the issue.

However, if I enable Clean URLs again with this patch applied, all non-front page links work fine (http://example.com/admin) but the <front> link (http://example.com/) still fails with the above error…

Comment #6

Damien Tournoud CreditAttribution: Damien Tournoud commented 1 March 2010 at 08:24

Status:

Needs review

» Closed (won't fix)

The url() function tries to handle the situation of a web server where index.php isn't setup as a default document and clean urls are not in use with the following code at the end of the function...

Not really. The script is added because:

// On some web servers, such as IIS, we can't omit "index.php". So, we
// generate "index.php?q=foo" instead of "?q=foo" on anything that is not
// Apache.

(as documented above in url())

We don't and can't support the situation where index.php is not in the list of default documents. Tentatively won't fixing, but we might want to add a small code comments in the bottom of url(), or move all the $script-related code there.

Comment #7

effulgentsia CreditAttribution: effulgentsia commented 1 March 2010 at 16:56

Title:	url('<front>') on IIS broken	» Document why index.php is added to urls with query string, but not to front page
Component:	base system	» documentation
Status:	Closed (won't fix)	» Needs work

We don't and can't support the situation where index.php is not in the list of default documents.

Why not? Needs documentation.

HEAD's url() function:

// On some web servers, such as IIS, we can't omit "index.php". So, we
// generate "index.php?q=foo" instead of "?q=foo" on anything that is not
// Apache.
$script = (strpos($_SERVER['SERVER_SOFTWARE'], 'Apache') === FALSE) ? 'index.php' : '';
...
// Without Clean URLs.
...
if ($query) {
  return $base . $script . '?' . drupal_http_build_query($query) . $options['fragment'];
}
else {
  return $base . $options['fragment'];
}

On what web server that has index.php as a default document, can you not have http://example.com/?q=foo? Code comment just says "we can't omit index.php". It does not say why. As far as I know, if you have index.php as a default document, then you don't need $script in any situation, and if you don't have index.php as a default document, then you need $script whether you have a "q=" or not. Perhaps I'm wrong, so some documentation explaining the situation would be helpful.

Comment #8

Damien Tournoud CreditAttribution: Damien Tournoud commented 1 March 2010 at 17:26

This special case (ie. cannot use /?q=, have to use /index.php?q=) was written ages ago. If IIS now supports /?q=, we might want to remove this.

Comment #9

effulgentsia CreditAttribution: effulgentsia commented 2 March 2010 at 02:19

Component:

documentation

» base system

Moving back to "base system" to get some more clarity, even if all that ends up changing is code comments.

We don't and can't support the situation where index.php is not in the list of default documents.

Why not? I understand that with clean urls enabled, the front page should be http://example.com, and whether through url rewriting or default document or something, the web server needs to map that to index.php. But with clean urls disabled, why would you want to place an extra requirement on the server to have index.php as a default document (when we know that IIS does not ship with that setting), when simply changing the front page url to explicitly have index.php, making it consistent with all other Drupal urls, would remove the need for that requirement?

Comment #10

Damien Tournoud CreditAttribution: Damien Tournoud commented 6 April 2010 at 23:29

But with clean urls disabled, why would you want to place an extra requirement on the server to have index.php as a default document (when we know that IIS does not ship with that setting), when simply changing the front page url to explicitly have index.php, making it consistent with all other Drupal urls, would remove the need for that requirement?

Simply because in that case, you cannot omit the index.php for the frontpage either. This is unsupported on any web server we support, I don't see why we would make an exception for IIS here.

Comment #11

effulgentsia CreditAttribution: effulgentsia commented 7 April 2010 at 00:54

Title:	Document why index.php is added to urls with query string, but not to front page	» Remove hard-coded check for "Apache" for deciding whether to add "index.php" within url()
Category:	bug	» task
Status:	Needs work	» Needs review

File	Size
url-fix_script.patch	2.99 KB

I'm hoping this one can make everyone happy.

1. See #8. As far back as IIS 5.1, if "index.php" is listed as a default document, then you can have http://example.com/?q=... So current HEAD code of outputting index.php when there is a query but not when there isn't makes no sense.

2. Hard-coded check for "Apache"? Come on.

3. Would be nice to make the original use-case of this issue work. Sometimes people need to deploy micro-sites to corporate IIS servers for which index.php can't be added as a default document.

This patch tries to achieve all goals.

Comment #12

effulgentsia CreditAttribution: effulgentsia commented 7 April 2010 at 16:14

File	Size
url-fix_script-437228-12.patch	2.65 KB

re-roll for HEAD changes + minor tweaking.

Comment #13

Dries CreditAttribution: Dries commented 7 April 2010 at 17:21

In general, I'm OK with this patch but I found the PHPdoc to be so-so.

+++ includes/common.inc	7 Apr 2010 16:12:32 -0000
@@ -1946,6 +1946,17 @@ function format_username($account) {
+ *   - 'script': Only used internally, to help some servers find 'index.php'

I don't understand why we write that it is only used internally? That seems to be redundant information as it is a public setting.

+++ includes/common.inc	7 Apr 2010 16:12:32 -0000
@@ -1946,6 +1946,17 @@ function format_username($account) {
+ *     servers are configured to automatically find index.php. Even IIS is

The PHPdoc starts of with a lot of information and assumes that the reader knows that Drupal uses index.php.

Powered by Dreditor.

Comment #14

effulgentsia CreditAttribution: effulgentsia commented 9 April 2010 at 18:15

I don't understand why we write that it is only used internally?

I have a question about that too. What do we mean when we say in the same PHPDoc in HEAD that the 'base_url' and 'prefix' options are only used internally? What I thought was meant is that they can be set within a hook_url_outbound_alter() implementation, but they should not be passed by the caller of url() (not that anything necessarily breaks if the caller passes them, just that these are options that shouldn't have any meaning to the caller). The same is true for 'script', but I agree that 'internal' might not be the best word to convey this concept.

Comment #15

sun

German

Karlsruhe

CreditAttribution: sun commented 9 April 2010 at 23:14

/**
 *   - 'script': The script filename in Drupal's root directory to use when
 *     clean URLs are disabled, such as 'index.php'. Defaults to an empty
 *     string, as most modern web servers automatically find 'index.php'. If
 *     clean URLs are disabled, the value of $path is appended as query
 *     parameter 'q' to $options['script'] in the returned URL. When deploying
 *     Drupal on a web server that cannot be configured to automatically find
 *     index.php, then hook_url_outbound_alter() can be implemented to force
 *     this value to 'index.php'.
 */