Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{{#show:}} calls can't find data from pages if first letter is not capitalized #3587

Closed
FO-nTTaX opened this issue Jan 9, 2019 · 5 comments
Labels
bug Occurrence of an unintended or unanticipated behaviour that causes a vulnerability or fatal error
Milestone

Comments

@FO-nTTaX
Copy link
Contributor

FO-nTTaX commented Jan 9, 2019

Setup and configuration

  • SMW version: 3.0.0
  • MW version: 1.31
  • PHP version: 7.0.33-0+deb9u1 (fpm-fcgi)
  • DB system (MySQL, Blazegraph, etc.) and version: 10.2.18-MariaDB-10.2.18+maria~stretch

Issue

I have a page at https://liquipedia.net/leagueoflegends/Mousesports (our MediaWiki settings have us uppercase all first letters). The canonical name of the page subject is however all lowercase, so mousesports. If I do a {{#show:mousesports|?Has image|default=File:Default.png|link=none}} ( https://liquipedia.net/leagueoflegends/User:FO-nTTaX/Sandbox/6 ) SMW does not find the correct data. This used to work in the 2.5 series which we were on before. For wikis that do not have the setting for lowercase first letters enabled, forcing the first letter uppercase should probably stay default, as this is what happens to all titles in MediaWiki.

Steps to reproduce

@mwjames
Copy link
Contributor

mwjames commented Jan 9, 2019

Create page with some properties like https://liquipedia.net/leagueoflegends/Mousesports
Use {{#show:}} with not ucfirst version of page name https://liquipedia.net/leagueoflegends/User:FO-nTTaX/Sandbox/6

I did a quick test:

  • Created Mousesports with [[Has test::123]] {{DISPLAYTITLE:mousesports}}
  • Created Mousesports/Test with {{#show: mousesports |?Has test |intro=mousesports: }} and {{#show: Mousesports |?Has test |intro=Mousesports: }}

and as expected only {{#show: Mousesports |?Has test |intro=Mousesports: }} returns a result, so for now I cannot reproduce the issue (and I'm pretty sure we have an integration test for this) and unless you changed the wgCapitalLinks settings back and forth, #ask (== #show) choose the canonical form (Mousesports) for a SMW_CMP_EQ condition independent of the DISPLAYTITLE or DEFAULTSORT (you may check the entity ID in smw_object_ids table). format=debug will produce a query output that contains something like t0.smw_id='18520' to match an ID and not some string field like smw_title or smw_sortkey.

This used to work in the 2.5 series which we were on before.

I'm cannot recall any changes that would neglect the canonical form (given that wgCapitalLinks = true).

@FO-nTTaX
Copy link
Contributor Author

FO-nTTaX commented Jan 10, 2019

We did not change $wgCapitalLinks, so that can't be it. Using the {{#show:mousesports|?Has image|default=File:Default.png|link=none}} call definitely used to work before to grab data from [[Mousesports]].

The debug format gives me this message:

ID select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE               Impossible WHERE noticed after reading const tables

The line you pointed out reads t0.smw_id='0'

@mwjames
Copy link
Contributor

mwjames commented Jan 10, 2019

I narrowed it down and it relates to #3325 where we had to change the caller because of Title::getUserCaseDBKey being removed (or is planned to be removed) and since we have to keep watch over wgCapitalLinks we missed setting the context in:

@@ -11,10 +11,11 @@ use SMW\Query\Language\Conjunction;
 use SMW\Query\Language\Description;
 use SMW\Query\Language\Disjunction;
 use SMW\Query\Language\ValueDescription;
 use SMW\Site;
 use SMWDataValue as DataValue;
+use SMW\Query\QueryComparator;
 
 /**
  * @license GNU GPL v2+
  * @since 2.4
  *
@@ -165,10 +166,24 @@ class DescriptionProcessor {
 		// the factory instance
 		$dataValue = $this->dataValueFactory->newTypeIDValue( '_wpg', 'QP_WPG_TITLE' );
 		$dataValue->setContextPage( $this->contextPage );
 
 		$dataValue->setOption( DataValue::OPT_QUERY_CONTEXT, true );
+
+		// #3587
+		// Requesting capital links is influenced by two factors, `wgCapitalLinks`
+		// is sitewide enabled and the condition for a `WikiPageValue` is
+		// identified to be SMW_CMP_EQ/NEQ (e.g. [[Foo]], [[!Foo]]) with other
+		// expressions (e.g. [[~foo*]]) remaining in the form of the user input
+		$queryComparator = QueryComparator::getInstance();
+
+		if ( Site::isCapitalLinks() && (
+			$queryComparator->containsComparator( $chunk, SMW_CMP_EQ ) ||
+			$queryComparator->containsComparator( $chunk, SMW_CMP_NEQ ) ) ) {
+			$dataValue->setOption( 'isCapitalLinks', true );
+		}

(and I'm pretty sure we have an integration test for this)

We have a test (P0427 [0]) which checks for [[~ab c*]] (== SMW_CMP_LIKE condition) but we don't have one for SMW_CMP_EQ and is the reason why we haven't detected the issue.

We need to write appropriate tests before we can continue and as things are right now, the to-do list is getting longer hence I'm unable to set a specific date as to when I'll be able to attend this task.

Tasks

  • Create a PR
  • Adapt failing tests (e.g. DescriptionProcessorTest)
  • Write a new P0461 test that covers above example
  • Verify that before the PR is applied the test fails and after the changes have been added the test passes

[0] https://github.com/SemanticMediaWiki/SemanticMediaWiki/blob/master/tests/phpunit/Integration/JSONScript/TestCases/p-0427.json#L32

@kghbln kghbln added the bug Occurrence of an unintended or unanticipated behaviour that causes a vulnerability or fatal error label Jan 12, 2019
@kghbln kghbln added this to the SMW 3.0.1 milestone Jan 12, 2019
@mwjames
Copy link
Contributor

mwjames commented Jan 12, 2019

SMW 3.0.1 milestone an hour ago
Create a PR

I'll see if I can squeeze some time during the upcoming weekend sprint.

@FO-nTTaX
Copy link
Contributor Author

Thank you so much for doing this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Occurrence of an unintended or unanticipated behaviour that causes a vulnerability or fatal error
Projects
None yet
Development

No branches or pull requests

3 participants