Special:Diff: Bad value for parameter $title: invalid name 'Diff/1_'
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Assigned To

Authored By

	Daimona
	Nov 26 2019, 6:57 PM

Description

Error

MediaWiki version: 1.35.0-wmf.5

message

[Xd1zIQpAAD0AAH@TyZMAAACC] /wiki/Speciale:Diff   Wikimedia\Assert\ParameterAssertionException from line 63 of /srv/mediawiki/php-1.35.0-wmf.5/vendor/wikimedia/assert/src/Assert.php: Bad value for parameter $title: invalid name 'Diff/1_'

Impact

The page fatals. Instead, it should fail gracefully and inform the user that the input is invalid (or trim the input).

Notes

Can be triggered by heading to Special:Diff, then paste something ending with a space (e.g. "1 ") in "Revision ID of difference" and hit "Show differences". As I wrote above, it should either fail gracefully, or (probably better) trim the input.

Details

Request ID: Xd1zIQpAAD0AAH@TyZMAAACC
Request URL: https://it.wikipedia.org/wiki/Speciale:Diff

Stack Trace

exception.trace

#0 /srv/mediawiki/php-1.35.0-wmf.5/includes/title/TitleValue.php(125): Wikimedia\Assert\Assert::parameter(boolean, string, string)
#1 /srv/mediawiki/php-1.35.0-wmf.5/includes/title/TitleValue.php(89): TitleValue::assertValidSpec(integer, string, string, string)
#2 /srv/mediawiki/php-1.35.0-wmf.5/includes/specialpage/SpecialPage.php(102): TitleValue->__construct(integer, string, string)
#3 /srv/mediawiki/php-1.35.0-wmf.5/includes/specialpage/SpecialPage.php(85): SpecialPage::getTitleValueFor(string, string, string)
#4 /srv/mediawiki/php-1.35.0-wmf.5/includes/specialpage/SpecialPage.php(673): SpecialPage::getTitleFor(string, string)
#5 /srv/mediawiki/php-1.35.0-wmf.5/includes/specials/SpecialDiff.php(98): SpecialPage->getPageTitle(string)
#6 /srv/mediawiki/php-1.35.0-wmf.5/includes/htmlform/HTMLForm.php(694): SpecialDiff->onFormSubmit(array, OOUIHTMLForm)
#7 /srv/mediawiki/php-1.35.0-wmf.5/includes/htmlform/HTMLForm.php(586): HTMLForm->trySubmit()
#8 /srv/mediawiki/php-1.35.0-wmf.5/includes/htmlform/HTMLForm.php(601): HTMLForm->tryAuthorizedSubmit()
#9 /srv/mediawiki/php-1.35.0-wmf.5/includes/specials/SpecialDiff.php(87): HTMLForm->show()
#10 /srv/mediawiki/php-1.35.0-wmf.5/includes/specials/SpecialDiff.php(69): SpecialDiff->showForm()
#11 /srv/mediawiki/php-1.35.0-wmf.5/includes/specialpage/RedirectSpecialPage.php(52): SpecialDiff->showNoRedirectPage()
#12 /srv/mediawiki/php-1.35.0-wmf.5/includes/specialpage/SpecialPage.php(575): RedirectSpecialPage->execute(NULL)
#13 /srv/mediawiki/php-1.35.0-wmf.5/includes/specialpage/SpecialPageFactory.php(607): SpecialPage->run(NULL)
#14 /srv/mediawiki/php-1.35.0-wmf.5/includes/MediaWiki.php(298): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#15 /srv/mediawiki/php-1.35.0-wmf.5/includes/MediaWiki.php(967): MediaWiki->performRequest()
#16 /srv/mediawiki/php-1.35.0-wmf.5/includes/MediaWiki.php(530): MediaWiki->main()
#17 /srv/mediawiki/php-1.35.0-wmf.5/index.php(46): MediaWiki->run()
#18 /srv/mediawiki/w/index.php(3): require(string)
#19 {main}

	Subject	Repo	Branch	Lines +/-
	Allow to select next/prev/cur on Special:Diff	mediawiki/core	master	+24 -2

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved	PRODUCTION ERROR	Umherirrender	T239265 Special:Diff: Bad value for parameter $title: invalid name 'Diff/1_'
		Declined		Ammarpad	T239452 Add new field to return trimmed text

Event Timeline

Daimona created this task.Nov 26 2019, 6:57 PM

Restricted Application added a project: Wikidata. · View Herald TranscriptNov 26 2019, 6:57 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Daimona edited projects, added MediaWiki-Special-pages; removed Wikidata, MediaWiki-extensions-Diff.Nov 26 2019, 6:58 PM

The other field (for the oldid) doesn't have this problem because it has 'type=number'. I wonder whether the diff field should be of type number as well, but I guess there's a reason if it isn't.

Also, I vote for trimming the input -- I don't see any point in failing for just an extra space. In case someone is interested in submitting a fix, this is where the input should be trimmed.

In T239265#5694758, @Daimona wrote:

The other field (for the oldid) doesn't have this problem because it has 'type=number'. I wonder whether the diff field should be of type number as well, but I guess there's a reason if it isn't.

Is there any situation where rev id is not an integer? The reason may well be that "it is because it is like that" since from the beginning.

Also, I vote for trimming the input -- I don't see any point in failing for just an extra space. In case someone is interested in submitting a fix, this is where the input should be trimmed.

The two fields should not be behaving differently. They should either be accepting int only (the only valid data) or do the trimming together. I'd go for the former.

Is there any situation where rev id is not an integer?

Yes, I just realized that it can be "prev" or "next". Of note, those fields are unchanged since the beginning.

And, BTW, the value isn't validated, so you can also insert "foobar" in that field and get a valid response.

The HTML form has a ton of field classes but does not have any that provides trimming. I think the proper way is to write a new class (like extension of HTMLTextField::class) that does trimming then use it, instead of doing some hacky things there. This way other code building HTML form can also use it if trimmed input is needed. I will give it a try. If there's a better way however, I can follow that.

Ammarpad mentioned this in T239452: Add new field to return trimmed text.Nov 29 2019, 6:53 AM

Ammarpad added a subtask: T239452: Add new field to return trimmed text.Nov 29 2019, 8:10 AM

You could validate the input field on the server side and than add a error message when used with the wrong values.

Or adding a dropdownlist with the valid values + integer text field

You can use prev, next and cur for the previous, next and current revision respectively.

A dropdown list also allows to localize the labels on the form

In T239265#5702142, @Umherirrender wrote:

Or adding a dropdownlist with the valid values + integer text field

This seems a great idea. Together with trimming and validating input, of course.

Krinkle moved this task from Untriaged to Nov2019/1.35.wmf.5+ on the Wikimedia-production-error board.Dec 2 2019, 8:40 PM

1997kB moved this task from To triage to Special:Contributions / Special:DeletedContributions on the MediaWiki-Special-pages board.Dec 24 2019, 2:02 PM

1997kB moved this task from Special:Contributions / Special:DeletedContributions to To triage on the MediaWiki-Special-pages board.Dec 24 2019, 2:15 PM

Umherirrender mentioned this in T256425: Allow HTMLSelectOrOtherField to be used with NumberInputWidget.Jun 25 2020, 8:31 PM

Change 607874 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Allow to select next/prev/cur on Special:Diff

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

gerritbot added a project: Patch-For-Review.Jun 25 2020, 8:33 PM

@Umherirrender Can you share a screenshot of what it will look like when submitting someting like "1 "? (one with space)

Ammarpad reassigned this task from Ammarpad to Umherirrender.Jun 30 2020, 6:22 PM

In T239265#6268597, @Krinkle wrote:

@Umherirrender Can you share a screenshot of what it will look like when submitting someting like "1 "? (one with space)

The status quo is an exception formatted by mediawiki with the usually navigation elements from the skin around it.

My patch makes the "1 " working, because a space is also trimmed when it is inputted in the other field of the special page.

When there are letters in it then a error message is shown under the field (the position is from the html field).
The other field on that special page shows a nice message and does not allow to submit the special page because it is using the number type and the browser validates the input.
That needs another feature request to be processed: T256425: Allow HTMLSelectOrOtherField to be used with NumberInputWidget

Krinkle mentioned this in T256798: Add option to HTMLIntField that disallows plus/minus sign (unsigned, e.g. revision id).Jul 1 2020, 12:46 AM

Change 607874 merged by jenkins-bot:
[mediawiki/core@master] Allow to select next/prev/cur on Special:Diff

https://gerrit.wikimedia.org/r/c/mediawiki/core/ /607874

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.40; 2020-07-07).Jul 1 2020, 2:00 AM

Krinkle closed this task as Resolved.Jul 1 2020, 3:14 AM

Krinkle removed a project: Patch-For-Review.

Ammarpad closed subtask T239452: Add new field to return trimmed text as Declined.Aug 18 2020, 10:25 AM