From drupal-devel@drupal.org Fri Jul 1 13:21:47 2005 From: killes To: development@drupal.org Subject: [drupal-devel] [bug] request_uri() broken if in subdir Date: Fri, 01 Jul 2005 13:21:59 +0000 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5590310903391039316==" --===============5590310903391039316== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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@www.drop.org Updated by: killes@www.drop.org Status: patch Another manifestation of this bug: http://drupal.org/node/26163 killes@www.drop.org Previous comments: ------------------------------------------------------------------------ September 17, 2004 - 21:05 : killes@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@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@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@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@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 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: test [1] test [2] 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 |=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D |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 |=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D |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.pa= tch (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 --===============5590310903391039316==--