So, uhm, this is tricky.

Coming from #600392: search_form() has an $action argument but this argument is partially ignored:

With the new HTTPS support, but also in general, anything can turn a path into an absolute URL by calling url().

With clean URLs disabled, we get http://example.com/?q=foo/bar

drupal_parse_url() is supposed to return the proper 'path', 'query', and 'fragment' for a URL, suitable to pass that along to url().

However, above example will get you array('path' => 'http://example.com/', 'query' => array('q' => 'foo/bar')) instead of 'foo/bar' in 'path'.

The only idea I have to solve this is to check whether clean URLs are disabled, and conditionally overwrite the parsed 'path' in case 'query' contains a 'q'...

ugh.

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

sun’s picture

Status: Needs work » Needs review
StatusFileSize
new1.86 KB

And this is the only solution I can think of.

sun’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.99 KB

I take the blame for not accounting for this situation in the original patch that introduced drupal_parse_url().

webchick’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: -D7 API clean-up

I'd like to see someone else eyeball this fix. There are no APIs touched here; it's just a straight-up bug fix, which can happen anytime post-10/15.

Removing tag accordingly.

sun’s picture

I'm inclined to re-add the tag, because this fix is also required for #582456: Make confirm_form() respect query string destination

webchick’s picture

I don't understand how that would be the case. It'd just mean confirm forms would be broken for non-clean URLs as well.

sun’s picture

nah, the issue is that other patch is an API clean-up patch, which currently *cannot* pass tests, because drupal_parse_url() doesn't work correctly without clean URLs, and because the testbot is running without clean URLs. :)

damien tournoud’s picture

This makes a lot of sense. I would like to see the check for q= being made unconditional, because it's the way it works currently (at least in Apache).

sun’s picture

StatusFileSize
new2.84 KB

Removed the conditional check for clean URLs.

sun’s picture

StatusFileSize
new2.84 KB

Super-minor test clean-up.

damien tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Good. This makes drupal_parse_url() an intelligent, specialized, inverse of url().

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Thanks. Committed to HEAD!

Status: Fixed » Closed (fixed)
Issue tags: -API clean-up

Automatically closed -- issue fixed for 2 weeks with no activity.