From hero to zeroes

gomedia · 2021-06-24 02:53:39

Zeroes are not very good for names but bear with me and consider these:

Create a page with name “0” – it fails with red message “Page name is invalid” … fair enough
Create a page with name “00” – this succeeds with green message “Pages created: 00” … er, OK
Try to change the page name from “00” to “000” – get green message “Pages updated: 000” … however, although the ‘All pages’ list shows “000”, the actual ‘Page name’ field shows “00”
Renaming from “000” to “0000” displays the same symptoms

Yes, I know this is a obscure/never likely scenario but it might highlight a strings/numbers issue in the code somewhere.

etc · 2021-06-24 07:02:58

Amazing. The first point is coming from a loose !$newpage test and is easy to patch, but the next ones are obscure. Thanks for the report.

etc · 2021-06-24 08:13:57

Should be patched for Pages in 4.8.8, but Forms and Styles are probably affected by the same problem.

gomedia · 2021-06-24 08:24:12

etc wrote #330648:

… Forms and Styles are probably affected by the same problem.

Yes, I should’ve mentioned. Forms & Styles misbehave as well.

etc · 2021-06-24 08:34:33

gomedia wrote #330649:

Yes, I should’ve mentioned. Forms & Styles misbehave as well.

Ok, if Pages are fine now, I will apply the same patch to these.

gomedia · 2021-06-24 11:35:16

etc wrote #330650:

Ok, if Pages are fine now, I will apply the same patch to these.

Looks good.

etc · 2021-06-27 16:12:47

Well, 0 Page is kinda useless, since any section associated with this page will be considered pageless. Dunno whether we should fix it, and in how many places…

gomedia · 2021-06-27 22:06:18

etc wrote #330753:

Well, 0 Page is kinda useless, since any section associated with this page will be considered pageless. Dunno whether we should fix it, and in how many places…

Back at the start of all this I wasn’t surprised that a Page name of “0” was disallowed.

However, if a name passes the input $nameRegex then why should it be blocked on Save?

I suspect zero was originally disallowed due to it being traditionally “false” in PHP. But if zero is a special value for Pages elsewhere, or it’s too dangerous (or onerous) to let through then, by all means, disallow it – but perhaps it should be trapped on input rather than at the second line of defence?

Bloke · 2021-06-27 22:15:59

Page/Form/Style or, in fact, anything with a name equating to ‘0’ should probably be disallowed. It means extra diligence in checks, and can easily be mistaken for empty or false, so I’d rather just ban it outright. It makes little sense as an asset name.

Plus, in the upcoming unlimited custom fields implementation, a ‘0’ row is used (internally) as a special marker in the cf-to-data mapping table for registering ‘the default value’ of a custom field so we shouldn’t allow it to be used as a CF name there as it might interfere or, at the very least, be confusing for debugging.

Last edited by Bloke (2021-06-27 22:18:15)

etc · 2021-06-28 07:34:04

Bloke wrote #330757:

Page/Form/Style or, in fact, anything with a name equating to ‘0’ should probably be disallowed. It means extra diligence in checks, and can easily be mistaken for empty or false, so I’d rather just ban it outright. It makes little sense as an asset name.

I agree.

etc · 2021-06-28 14:30:16

gomedia wrote #330756:

But if zero is a special value for Pages elsewhere, or it’s too dangerous (or onerous) to let through then, by all means, disallow it – but perhaps it should be trapped on input rather than at the second line of defence?

Maybe done for Pages, please test again.

Bloke · 2021-06-28 15:32:02

Nice, thanks. Presumably, the $namePattern regex in Textpattern/Skin/CommonBase.php is okay still, as that governs the exported asset name validity (I think). We should also test if it’s possible to create a page in a disk-based theme with the name 0.txp and ensure it can’t be imported, if anyone fancies trying that out.

Textpattern CMS

Textpattern CMS support forum

#1 2021-06-24 02:53:39