If I have a CCK text field as a select that I'm using for faceted search, if the select key has a space in it (which is easy if you don't use a | to set a key and the user-facing value has a space) then filtering on that value fails.

CommentFileSizeAuthor
#5 space-prob-364446-5.patch4.56 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented

It looks like we should already handle this?

  static function make_field(array $values) {
    if (empty($values['#name'])) {
      return implode(' ', array_filter(explode(' ', $values['#value']), 'trim'));
    }
    else {
      // if the field value has spaces, or : in it, wrap it in double quotes.
      if (preg_match('/[ :]/', $values['#value'])) {
        $values['#value'] = '"'. $values['#value']. '"';
      }
      return $values['#name'] . ':' . $values['#value'];
    }
  }
pwolanin’s picture

pwolanin commented

ok, there is some breakage when building the query - echoing the actual query it looks like:

&fq=fieldname:"very&version=1.2&wt=json

when it shoudl be:

&fq=fieldname:"very bad"&version=1.2&wt=json

pwolanin’s picture

pwolanin commented

Ah rats - somewhere we are getting encoding - the pattern never matches!

I see the actual string is:

fieldname:"very bad"

pwolanin’s picture

pwolanin commented

aaargh:

  if (empty($filters) && !empty($_GET['filters'])) {
    $filters = check_plain($_GET['filters']);
  }
pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new space-prob-364446-5.patch4.56 KB

It's not clear we need to check_plain either of these... ?

There is still a problem with CCK values containing a "

robertdouglass’s picture

robertdouglass commented

I'm not aware of any places where we show stuff from the URL in the browser without filtering, so the check_plain on the filters looks superfluous. Haven't evaluated the patch, but if it moves towards fixing the feature set, +1.

pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed

committed to 6.x

Status: Fixed » Closed (fixed)

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