Page MenuHomePhabricator

Setting query string values for multiple-instance templates leads to an infinite loop
Closed, ResolvedPublic

Description

MW 1.33 et PF 5.1+

When trying to create a new page thanks to {{#formlink which has many query strings the form get time out. The form is trying to load many values with many

values from (Property|Category)

The bug may come from this commit

https://phabricator.wikimedia.org/rEPFMd29c9f37d23d1aea8b658148ce86c6df6b6bef4d

and maybe this new statement (line 1095) :

$page_value = $tif->getValuesFromPage()[$field_name];

Event Timeline

What do you mean by "with #formlink"? #formlink just defines a link. Do you mean that the form that #formlink links to is timing out? If so, what is different about this form than other forms that are working fine? Is it the fact that this form uses the "one-step process" instead of the "two-step process"?

Indeed, the #formlink itself correctly works and is not an issue. The issue is when we click on such link, and specifically when there are multiple-instance templates. I add the wiki in question has a lot of semantic data, perhaps it is not reproductible on a smaller wiki.
We really isolated the observed timeout on this commit d29c9f3 in the sense that there is no timeout with PF 5.1 with a git-revert of this commit (details: git checkout 5.1 && git revert -Xours --no-edit d29c9f3).

Okay, that's very helpful. So if you comment out that line (line 1095), the form gets displayed?

I spent some time yesterday trying to understand but it’s not finished. A partial conclusion is that the said line 1095 is perhaps not an issue because I observed the value of the variable $tif during a few (150) iterations (we are in a for-loop then a while-loop) and these calls works. IIRC I commented the line and the issue was still here, but I will confirm. I will continue to investigate this next week.

If you're still stuck, please try this: in the file /includes/PF_TemplateInForm.php, in the function incrementInstanceNum() (around line 263), add the following line:

if ( $this->mInstanceNum > 100 ) die("Error! Over 100 instances.");

If the issue is actually an infinite loop, this will hopefully uncover it.

It is indeed an infinite loop (I added the die where you said), reproductible on PF 5.1 with the following form:

{{{info|page name=Page<unique number;start=000001>}}}
{{{for template|MyTemplate|multiple|add button text=Add a template}}}
Input : {{{field|SomeField|input type=text}}}
{{{end template}}}
----
{{{standard input|save}}}
{{{standard input|cancel}}}

When calling the page /index.php?title=Special:FormEdit/MyTemplate&MyTemplate[1][SomeField]=ABC, which functions with standard 5.0 and with 5.1 without the said commit.

I remark /index.php?title=Special:FormEdit/MyTemplate&MyTemplate[0][SomeField]=ABC functions but the value is not pre-filled.

An hypothesis is a change in some counter.

I added a comment in rEPFMd29c9f37d23d1aea8b658148ce86c6df6b6bef4d about the faulty line – I tried to make a link to this line but it turns it created a comment, so I did it; I didn’t know this interface/procedure.

@Seb35 - thank you, this is very helpful. Is there a specific change/patch you would recommend that you think would fix this issue?

Yaron_Koren renamed this task from creating a new page with {{#formlink get a time out to Some query string values for multiple-instance templates lead to an infinite loop.Apr 9 2021, 1:43 PM
Yaron_Koren renamed this task from Some query string values for multiple-instance templates lead to an infinite loop to Setting query string values for multiple-instance templates lead to an infinite loop.Apr 9 2021, 3:56 PM
Yaron_Koren renamed this task from Setting query string values for multiple-instance templates lead to an infinite loop to Setting query string values for multiple-instance templates leads to an infinite loop.

Change 678098 had a related patch set uploaded (by Yaron Koren; author: Yaron Koren):

[mediawiki/extensions/PageForms@master] Fix for d29c9f37d23d - caused infinite loop for some query string vals

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

Change 678098 merged by jenkins-bot:

[mediawiki/extensions/PageForms@master] Fix for d29c9f37d23d - caused infinite loop for some query string vals

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

Yaron_Koren claimed this task.

I think this is now fixed - feel free to re-open if not.