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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
1.86 KB

And this is the only solution I can think of.

sun’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
2.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

Removed the conditional check for clean URLs.

sun’s picture

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.