Page MenuHomePhabricator

RESTBase requests to Parsoid/PHP that contain a "." in the title (without a /<revid> component) fail with a http 403
Closed, ResolvedPublic

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMon, Dec 2, 10:15 PM
Jdforrester-WMF triaged this task as Unbreak Now! priority.Mon, Dec 2, 10:16 PM
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptMon, Dec 2, 10:16 PM
cscott added a subscriber: cscott.

Tagging RESTBase as well, just in case it's a bug in the middle layer.

(Might actually be a bug in core w/ integer titles? https://gerrit.wikimedia.org/r/554122 was failing parsertests in core, not in parsoid...)

(Might actually be a bug in core w/ integer titles? https://gerrit.wikimedia.org/r/554122 was failing parsertests in core, not in parsoid...)

Unlikely, T239645 is only about Postgres

Tagging RESTBase as well, just in case it's a bug in the middle layer.

This is a good thought. A quick test on my local wiki without RESTBase seemed to go through

I managed to create
https://en.wikipedia.org/w/index.php?title=42_Test_for_T239666&action=history
with the new wikitext editor, and
https://en.wikipedia.org/w/index.php?title=42_Test_for_T239666_2&action=history
with visual editor and didn't see any problems.

So I think it's stronger than "starts with digits", I think the title must be a valid number with no extraneous characters.

cscott renamed this task from Trying to create a page whose title starts with digits causes PHP Parsoid(?) to return 403s to Trying to create a page whose title is a number (with no non-number chars) causes Parsoid/PHP(?) to return 403s.Mon, Dec 2, 10:41 PM

This is a good thought. A quick test on my local wiki without RESTBase seemed to go through

I setup RESTBase locally and a test with it passed too, so I'm probably not testing this right

ssastry lowered the priority of this task from Unbreak Now! to High.Tue, Dec 3, 2:49 AM
ssastry added a subscriber: ssastry.

Lowering priority since this is en edge case in an uncommon use case.

No.

I managed to create
https://en.wikipedia.org/w/index.php?title=42_Test_for_T239666&action=history
with the new wikitext editor, and
https://en.wikipedia.org/w/index.php?title=42_Test_for_T239666_2&action=history
with visual editor and didn't see any problems.
So I think it's stronger than "starts with digits", I think the title must be a valid number with no extraneous characters.

No. See the initial report, which is about a title with non-digit characters.

This is nothing like as much of an edge case.

Related to T232556 because all of @Jdforrester-WMF's examples have a . in them?

ssastry renamed this task from Trying to create a page whose title is a number (with no non-number chars) causes Parsoid/PHP(?) to return 403s to Trying to create a page that contains a . causes Parsoid/PHP(?) to return 403s.Tue, Dec 3, 3:13 AM
ssastry updated the task description. (Show Details)
ssastry edited subscribers, added: mobrovac; removed: Parsing-Team.EditedTue, Dec 3, 3:24 AM

My hunch: if RESTBase uses the T232556 workaround of appending a "/" to the url it posts to Parsoid/PHP, this will be fixed. @mobrovac FYI.

Change 554227 had a related patch set uploaded (by Tim Starling; owner: Tim Starling):
[mediawiki/core@wmf/1.35.0-wmf.8] [1.35.0-wmf.8] Remove checkUrlExtension() from REST API

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

ssastry renamed this task from Trying to create a page that contains a . causes Parsoid/PHP(?) to return 403s to RESTBase requests to Parsoid/PHP that contain a "." in the title (without a /<revid> component) fail with a http 403.Tue, Dec 3, 3:46 AM
ssastry added a subscriber: Sebastian_Berlin-WMSE.

Try editing https://en.wikipedia.org/wiki/97.7_FM in the new wikitext source editor and preview. It fails with a 403. T239456 failures are also because of the same reason (all file pages have a ".jpg" or some such extension to them).

Anyway, the solution here is one of the following three:
(a) wait for https://gerrit.wikimedia.org/r/c/mediawiki/core/+/552679 to ride the train this week -- probably too long
(b) @tstarling was proposing backporting some piece of that
(c) RESTBase tacks on a trailing "/" to its requests that don't have a revid (new content pages, previews)

This is a good thought. A quick test on my local wiki without RESTBase seemed to go through

I setup RESTBase locally and a test with it passed too, so I'm probably not testing this right

As is clear from this discussion, either you didn't use a "." in your titles and/or you had the latest mediawiki core code checked out.

Change 554228 had a related patch set uploaded (by Tim Starling; owner: Tim Starling):
[mediawiki/core@wmf/1.35.0-wmf.5] [1.35.0-wmf.5] Remove checkUrlExtension() from REST API

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

We are going with solution (b). As for (c), there was T235179: Implement workarounds in RESTBase and Flow to hit Parsoid/PHP REST API endpoints without an oldid for titles containing ".". I didn't make it a subtask of T229015 and lost track of it.

Change 554227 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.8] [1.35.0-wmf.8] Remove checkUrlExtension() from REST API

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

Change 554228 merged by jenkins-bot:
[mediawiki/core@wmf/1.35.0-wmf.5] [1.35.0-wmf.5] Remove checkUrlExtension() from REST API

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

Mentioned in SAL (#wikimedia-operations) [2019-12-03T04:15:38Z] <tstarling@deploy1001> Synchronized php-1.35.0-wmf.8/includes/Rest/EntryPoint.php: disable IE6 safety checks for T239666 (duration: 01m 01s)

Mentioned in SAL (#wikimedia-operations) [2019-12-03T04:19:10Z] <tstarling@deploy1001> Synchronized php-1.35.0-wmf.5/includes/Rest/EntryPoint.php: disable IE6 safety checks for T239666 (duration: 01m 00s)

ssastry closed this task as Resolved.Tue, Dec 3, 4:22 AM
ssastry assigned this task to tstarling.

With the backports deployed, verified fixed with (a) preview in the 2017 wte editor on enwiki:97.7FM that was previously failing (b) trying to create the new page on some of the urls in the description.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptTue, Dec 3, 4:22 AM