Just noticed that all of my watchdog location links are wrong, they say: /drupal_cvs/drupal/?q=node/941/edit but the link is actually http://localhost/drupal_cvs/drupal/?q=/drupal_cvs/drupal/%3Fq%3Dnode/941/edit ... eww, using the site root too many times it appears.

CommentFileSizeAuthor
#20 request_uri_3.patch5.03 KBchx
#19 request_uri_2.patch4.96 KBchx
#15 request_uri_1.patch1.39 KBchx
#12 request_uri_0.patch1.41 KBchx
#5 watchdog.module_0.patch1.53 KBm3avrck

Comments

kbahey’s picture

kbahey commented

I cannot reproduce this.

I tried with two sites, one is beta3 installed in a subdirectory, and the other is today's HEAD installed in documentroot.

Can you post what is in your $base_url from settings.php?

m3avrck’s picture

m3avrck commented

Yes I have latest HEAD too, very strange.

$base_url = 'http://localhost/drupal_cvs/drupal';

All of the location links are wrong (actually clicking the link that is). Hmmm, in the DB they are correct but some reason or other all of the parent folders are used twice. It seems it is a sub-directory issue, no?

kbahey’s picture

kbahey commented

From the looks of it, it seems that the part is duplicated:

?q=/drupal_cvs/drupal/?q=

Which theme are you using? Does it output a base href?

<base href="YOURBASEURL" />

If all else fails, try this:

- Download or checkout the latest Drupal HEAD.
- drop all tables in the database.
- Reinstall Drupal at the same location, with the same $base_url.
- Create the database.
- Check if watchdog still has an issue.

m3avrck’s picture

m3avrck commented

Using default bluemarine theme.

<base href="http://localhost/drupal_cvs/drupal/" />

I'll try clearing my database and what not soon enough, I just did this a few days ago, I still think this might be a sub-dir issue with either l() or url() or similar. Tried tracking it last night but good to tired, I'll look into this more soon.

m3avrck’s picture

m3avrck commented
Status: Active » Needs review
StatusFileSize
new watchdog.module_0.patch1.53 KB

Ok here's a patch to fix this. Seems the problem stems from the fact that request_uri() will return say: '/drupal_cvs/drupal/?q=admin/node' and this is stored in $watchdog->location. However, this is then passed to l() where that whole string is appended to $base_url which causes this funky behavior.

Tested and working correctly now on Apache 2.0.55.

drewish’s picture

drewish commented

I don't think this patch will work. I'm running into this with clean URLs enabled and all my links end up like /node/231, the leading slash makes it useless. The base URL needs to be appended in some place. I'm going to do a little digging and see if I can get a patch working.

drewish’s picture

drewish commented

see also: http://drupal.org/node/49211

puregin’s picture

puregin commented

For some reason the offset of '+ 3' in the strpos() call seems to chop off too much when I tested this.

dries’s picture

dries commented
Status: Needs review » Needs work

I think we should change request_uri() to return a different URL. It should return URLs relative to the base url, not relative to the root.

This will also fix the use of request_uri() in the page caching.

markus_petrux’s picture

markus_petrux commented

hmmm... is this one related? a dup?
http://drupal.org/node/47255

m3avrck’s picture

m3avrck commented

Seems related to me.

And another one: http://drupal.org/node/41595

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new request_uri_0.patch1.41 KB

This was a hell to roll. I am afraid a bit that this will break already existing translations? Killes, please comment.

chx’s picture

chx commented

This little excerpt from IRC can't replace the full review which I hope will shortly follow, but from IRC talk it seems killes is OK with the change and it won't break locale. How OK he is with the change -- we shall see when he commits. To further that -- review please!

chx’s picture

chx commented

after this patch, we get the aliased drupal path + querystring in request_uri. I am not sure about the slash in front. I will give it some more thought...

chx’s picture

chx commented
Status: Needs review » Needs work
StatusFileSize
new request_uri_1.patch1.39 KB

For reasons unknown my system went over to rolling -p1 patches. I will investigate. Anyways... it seems that a) slash trimmign is not needed b) unclean urls are a problem -- the equal sign of ?q= is still encoded at watchdog. But it's progress, still.

chx’s picture

chx commented
Title: Watchdog location link wrong » fix up request_uri
Component: watchdog.module » base system
Assigned: Unassigned » chx
Category: bug » task
Priority: Minor » Critical
m3avrck’s picture

m3avrck commented

Hmm, still not working here.

Location ?q=logout

But that link is actually:

http://localhost/drupal_devel/drupal/?q=%3Fq%3Dlogout

Looks like it is using q= twice there.

Seems to work with clean URLs though.

m3avrck’s picture

m3avrck commented

This other issue I had, http://drupal.org/node/41595 , has issues with q= too in the l(). Semi related.

chx’s picture

chx commented
StatusFileSize
new request_uri_2.patch4.96 KB

it's easy and necessary to remove the double encoding. still we have two ?q=

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new request_uri_3.patch5.03 KB

Brute force patch which turns request_uri always into aliased drupalpath+querystring.

m3avrck’s picture

m3avrck commented

Almost working, however one problem with clean urls disabled.

http://localhost/drupal_devel/drupal/?q=node?destination=node

Seems it adds an extra ? instead of an &

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

Doesn't apply and m3avrck raised an issue.

chx’s picture

chx commented
Status: Needs work » Fixed

Steven fixed this in 4.7 already

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)