Page MenuHomePhabricator

Multi namespace search failed due to array normalization issue
Closed, ResolvedPublic1 Estimated Story PointsBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Configure additional NS to be searched, ex:
$wgNamespacesToBeSearchedDefault[NS_MODULE] = true;
$wgNamespacesToBeSearchedDefault[NS_MODULE_TALK] = true;
  • Use keyword search or request directly
  • /api.php?action=opensearch&format=json&formatversion=2&search=dance&namespace=0|828|829&limit=10

What happens?:

// error logged
// [1] [reason: [1:526] [bool] failed to parse field [filter]]

// empty response
[
  "dance",
  [

  ],
  [

  ],
  [

  ]
]

What should have happened instead?:
When query with multiple namespaces, query should not throw error and return proper results

Software version (skip for WMF-hosted wikis like Wikipedia):
REL1_39

Analysis & workaround
https://stackoverflow.com/a/369761
The problem is introduced in
extensions/CirrusSearch/includes/CompletionSuggester.php and has been there for years

foreach ( $namespaces as $k => $v ) {
    // non-strict comparison, it can be strings
    if ( $v === NS_MAIN ) {
        unset( $namespaces[$k] );
    }
}

if ( $namespaces === [] ) {
    return null;
}

But it was never an issue since 6.x Elastica has the following code

public function setTerms($key, array $terms)
{
    $this->_key = $key;
    $this->_terms = array_values($terms);

    return $this;
}

However, in Elastica 7.x the normalization is no longer applied, causing query generated to not be accepted by ES, bug filed here https://github.com/ruflin/Elastica/issues/2139

Temp workaround is to normalize the array in the extension itself

foreach ( $namespaces as $k => $v ) {
    // non-strict comparison, it can be strings
    if ( $v === NS_MAIN ) {
        unset( $namespaces[$k] );
        $namespaces = array_values($namespaces);
    }
}

if ( $namespaces === [] ) {
    return null;
}

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Gehel set the point value for this task to 1.Jan 9 2023, 4:53 PM

Change 877208 had a related patch set uploaded (by Ebernhardson; author: Ebernhardson):

[mediawiki/extensions/CirrusSearch@REL1_39] Ensure namespace filters is passed as a list

https://gerrit.wikimedia.org/r/877208

EBernhardson subscribed.

Thanks for looking into this, it seems this was fixed in the master branch but not backported into REL1_39. I've added a backport and also took a look over the patches merged since the REL1_39 branching to ensure we haven't missed any other 7.x compatability patches. It looks like this was the only patch missed.

Change 877208 merged by jenkins-bot:

[mediawiki/extensions/CirrusSearch@REL1_39] Ensure namespace filters is passed as a list

https://gerrit.wikimedia.org/r/877208