[drupal-devel] [bug] request_uri() broken if in subdir

killes drupal-devel at drupal.org
Fri Jul 1 13:21:59 UTC 2005


Issue status update for http://drupal.org/node/10917

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     bug reports
 Priority:     normal
 Assigned to:  Robin Monks
 Reported by:  killes at www.drop.org
 Updated by:   killes at www.drop.org
 Status:       patch

Another manifestation of this bug:
http://drupal.org/node/26163




killes at www.drop.org



Previous comments:
------------------------------------------------------------------------

September 17, 2004 - 21:05 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/requi.patch (678 bytes)

We asumme the $_SERVER[REQUEST_URI] would contain only the url from the
point of Drupal's directory on. This is true only iff you run from the
root of your site. If you are in a subdir, the function request_uri()
will be unusable if we want to construct a valid url from $base_url and
request_uri() (what we do in a number of places). The attached patch
fixes this by using basename().




------------------------------------------------------------------------

September 17, 2004 - 21:45 : killes at www.drop.org

The patch doesn't suffice if your access only the base_url of your
Drupal site in a subdir. Putting on hold.




------------------------------------------------------------------------

September 17, 2004 - 23:39 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/request.patch (761 bytes)

ok, I think I got it. The distinction between apache and non-apache
servers went away to keep it simple.




------------------------------------------------------------------------

September 18, 2004 - 02:19 : killes at www.drop.org

Apparently the action in forms depends on the bug that I found, ie it
needs request_uri to contain the full url (from documentroot on).




------------------------------------------------------------------------

September 18, 2004 - 02:53 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/request_0.patch (2.46 KB)

This new patch fixes the problem with forms. request_uri() also does not
start with a leading slash anymore. Ask Steven for why this is good.
http://acko.net/dumpx/relative.html is apparently a clue I don't get.




------------------------------------------------------------------------

September 18, 2004 - 03:04 : Steven

Explanation:


People are going to stick relative_uri() into links. These links are
resolved relative to the $base_url which is put into the <head> tag.
However, if a link URI starts with a / then it is relative to the root
of the site, even if the base tag uses a path with a subdirectory.


Example:
<html>
<base href="http://www.test.com/subdir1/">
<body>
test [1]
test [2]
</body>
</html>


The first link resolves to http://www.test.com/subdir2, the second
resolves to http://www.test.com/subdir1/subdir2.


Thus, it is more meaningful to not have a leading slash, as this will
avoid possible bugs with subdirs (which often do not get caught because
the tester uses a site without a subdir) and hints at the callee that
this is a relative URI too.


[1] http://drupal.org//subdir2
[2] http://drupal.org/subdir2




------------------------------------------------------------------------

March 13, 2005 - 20:38 : Chris Johnson

Patch no longer applies completely to bootstrap.inc:


Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: includes/bootstrap.inc
|===================================================================
|RCS file: /cvs/drupal/drupal/includes/bootstrap.inc,v
|retrieving revision 1.28
|diff -u -F^f -r1.28 bootstrap.inc
|--- includes/bootstrap.inc     16 Sep 2004 07:17:54 -0000      1.28
|+++ includes/bootstrap.inc     18 Sep 2004 00:52:12 -0000
--------------------------
Patching file bootstrap.inc using Plan A...
Hunk #1 succeeded at 273 (offset 83 lines).
Hunk #2 succeeded at 292 (offset 83 lines).
Hunk #3 failed at 446.
1 out of 3 hunks failed--saving rejects to bootstrap.inc.rej
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: includes/common.inc
|===================================================================
|RCS file: /cvs/drupal/drupal/includes/common.inc,v
|retrieving revision 1.390
|diff -u -F^f -r1.390 common.inc
|--- includes/common.inc        16 Sep 2004 16:12:21 -0000      1.390
|+++ includes/common.inc        18 Sep 2004 00:52:12 -0000
--------------------------
Patching file common.inc using Plan A...
Hunk #1 succeeded at 988 (offset -90 lines).
done




------------------------------------------------------------------------

June 14, 2005 - 13:21 : Robin Monks

Attachment: http://drupal.org/files/issues/request_uri.broken.if.in.subdir.patch (2.6 KB)

Here is that patch re-rolled for latest CVS HEAD.


Since killes hasn't been active in this bug lately, I hope he won't
mind my reassigning this to myself.


Robin




------------------------------------------------------------------------

June 14, 2005 - 13:22 : Robin Monks

I've also tested this to work on CVS HEAD.


Robin




------------------------------------------------------------------------

June 14, 2005 - 15:33 : moshe weitzman

did you test this on non apache servers? the comment that you removed is
crystal clear about a need for 2 different cases. it is OK to 'make it
simpler' as long as you don't remove compatibility with other web
servers. I am primarily concerned with IIS.




------------------------------------------------------------------------

June 14, 2005 - 16:42 : Robin Monks

I took great care to ensure I didn't remove any support needed for other
servers.  I also tested this patch to work on Xitami on Win32.


Robin




------------------------------------------------------------------------

June 14, 2005 - 19:09 : Robin Monks

I spent some more time testing, and this broke some (perhaps all)
configuration forms.  It appears the / is needed after all.  Perhaps
killes will have better luck understanding this, the / is needed IMHO.


Robin







More information about the drupal-devel mailing list