Errors when Solr used in conjunction with SecurePages module

Hmm, perhaps we need to urlencode the facet links

subscribe

@gregor7777:

I don't have a ssl setup up and running right now. So could you please test this patch for url encoding?

@gregor7777

I made a mistake when using array_walk to encode the filters.
I attached a corrected version of my patch which works for me. Please test again.

Why do we need the urldecode? It seems like will decode when adding to $_GET?

@pwolanin:

You're right. I adjusted the patch and removed the additional urldecode.

While playing around with securepages module (which really has some open issues from my point of view) I created a setup where the query string got urlencoded twice.

I didn't test this with securepages because I don't have SSL setup, and don't feel like dealing with it, but it seems to have no effect on normal operation.

Committed.

Same strategy for building querystrings as introduced with patch from comment #11 must be applied to breadcrumbs, too.
Because patch from comment #11 is already released, here's an additional one for breadcrumbs.

Looks reasonable

the committed patch uses drupal_urlencode() while this patch uses rawurlencode()

So, one (or both) of those must be wrong.

In fact, since this is for the query string (not path) that suggests we shoudl possibly be using: str_replace('%2F', '/', rawurlencode($text))

per the non-clean URL code in http://api.drupal.org/api/function/drupal_urlencode/6

per #310139: drupal_query_string_encode() should not call drupal_urlencode() and Damien, we shoudl just use rawurlencode(), which means the previously changed code needs to be similarly altered.

In fact, maybe we shoudl just do this?

$querystring = 'filters='. rawurlencode(implode(' ', $fq));

Wiat, now I'm at a loss as to why this is required. We are always passing our query string through url() which means that drupal_query_string_encode() is applied to all of them. What am I missing - it would seem like these changes give us double-encoding.

The patch from #11 used drupal_urlencode and was committed to CVS. Later drupal_urlencode seems to be replaced by rawurlencode in CVS. That's why the patch from #14 now uses rawurlencode, too.

@ #17 pwolanin:
replacing implode(' ', $fq) by implode('+', $fq) was one thing needed to solve the incompatibility to secure_pages module.

@ #18 pwolanin:
in our cases $options['query'] always seems to be an already prepared querystring and not an array, so drupal_query_string_encode() will not be called from url():

function url($path = NULL, $options = array()) {
  ...
  if (is_array($options['query'])) {
    $options['query'] = drupal_query_string_encode($options['query']);
  }

So far I can't see any double quoting here in my setup with patch #14 applied.

Ah, right - I see why it doesn't get encoded.

Try this patch, since ideally we would not use +

I remember that secure_pages chunks an url and than uses html_build_query() to reconstruct it internally.

print http_build_query(array('filters' => 'tid:315 tid:316'));

Output:

filters=tid%3A315+tid%3A316

can you check if the above patch works?

I'm having trouble testing this because when I enable SSL on my MAMP install. It:

1). Goes slow as hell
2). Fails to make outgoing HTTP requests.

I don't feel like dealing with finding out why, but the patch above doesn't seem to cause any problems when not using secure pages.

committed to 6.x CVS, follow up if it causes issues.

Sorry that I reopen this issue.

After reading through some specifications at w3c I'm of the opinion that spaces are not allowed in URLs and must be escaped.

"Spaces and control characters in URLs must be escaped for transmission in HTTP, as must other disallowed characters."
http://www.w3.org/Addressing/URL/url-spec.txt

According to the newer document http://labs.apache.org/webarch/uri/rfc/rfc3986.html in seciton "C. Delimiting a URI in Context" whitespaces delimit an url or must be ignored if they occur within "" or <>. Both treatments of whitespaces will break apachesolr urls.

From my point of view spaces must be escaped as '+'. And in addition to this issue about querystrings, spaces should also be avoided in search key words.

Was there a discussion about spaces in apachesolr urls earlier?

With this patch spaces ARE escaped as '%20', which is the standard for URLs, rather than as '+'.

If I look at your code you must be right. So it seems I made a mistake when applying your patch because I still saw spaces in breadcrumb in our test environment. Please excuse ...

As I wrote the first patch I decided to be compatible to urls created by securepages which used html_build_query(). Unfortunately I'm unable to check if %20 works until Monday.

What do you think about spaces between search key words? Shouldn't these be encoded as well?

The spaces between search keywords will by urlencoded by the Drupal url() function, so I think that's not an issue.

Seems most browsers display %20 decoded, so it may not be obvious when a link actually contains it.

let me know if this is not fixed.