Page MenuHomePhabricator
Create Task
Maniphest T239666

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

Description

Initial report: https://en.wikipedia.org/wiki/Wikipedia:VisualEditor/Feedback#Error_contacting_the_Parsoid/RESTBase_server_(HTTP_403)

E.g. try to create https://en.wikipedia.org/wiki/1.2.3.4?veaction=edit or https://en.wikipedia.org/wiki/97.72?veaction=edit

Not a problem if the page already exists, e.g. https://en.wikipedia.org/wiki/97.7_FM?veaction=edit

This seems related to T232556: Parsoid/PHP roundtrip testing: Period ( . ) in titles fail with a http 403 (Invalid file extension found in the path info or query string.) error. But now that IE6/IE7 support has been removed, it is possible this will unbreak once that code rides the train.

Details

SubjectRepoBranchLines +/-
[1.35.0-wmf.5] Remove checkUrlExtension() from REST APImediawiki/corewmf/1.35.0-wmf.5+0 -3
[1.35.0-wmf.8] Remove checkUrlExtension() from REST APImediawiki/corewmf/1.35.0-wmf.8+0 -3
Customize query in gerrit

Related Objects

Event Timeline

Jdforrester-WMF created this task.Dec 2 2019, 10:15 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 2 2019, 10:15 PM
Jdforrester-WMF triaged this task as Unbreak Now! priority.Dec 2 2019, 10:16 PM
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptDec 2 2019, 10:16 PM
cscott added a project: RESTBase.Dec 2 2019, 10:16 PM
cscott subscribed.

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

cscott added a comment.Dec 2 2019, 10:21 PM

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

cscott added a subscriber: Parsing-Team--ARCHIVED.Dec 2 2019, 10:21 PM
Arlolra subscribed.Dec 2 2019, 10:25 PM
In T239666#5707375, @cscott wrote:

(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

Arlolra added a comment.Dec 2 2019, 10:26 PM
In T239666#5707356, @cscott wrote:

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

cscott added a comment.Dec 2 2019, 10:39 PM

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.Dec 2 2019, 10:41 PM
Arlolra added a comment.Dec 2 2019, 11:49 PM
In T239666#5707391, @Arlolra wrote:

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.Dec 3 2019, 2:49 AM
ssastry subscribed.

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

Jdforrester-WMF added a comment.Dec 3 2019, 2:54 AM

No.

In T239666#5707419, @cscott wrote:

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.

Jdforrester-WMF updated the task description. (Show Details)Dec 3 2019, 2:55 AM
ssastry added a comment.Dec 3 2019, 3:08 AM

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

ssastry added a comment.Dec 3 2019, 3:11 AM

Yup. https://en.wikipedia.org/wiki/abcdefghijkl.mno?veaction=edit also fails.

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.Dec 3 2019, 3:13 AM
ssastry updated the task description. (Show Details)
ssastry edited subscribers, added: • mobrovac; removed: Parsing-Team--ARCHIVED.EditedDec 3 2019, 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.

• tstarling subscribed.Dec 3 2019, 3:24 AM

VisualEditor logs the failure response from RESTBase, e.g. https://logstash.wikimedia.org/app/kibana#/doc/logstash-*/logstash-mediawiki-2019.12.03/mediawiki?id=AW7JwbdXKWrIH1QRemDK&_g=h@44136fa

gerritbot added a comment.Dec 3 2019, 3:45 AM

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

gerritbot added a project: Patch-For-Review.Dec 3 2019, 3:45 AM
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.Dec 3 2019, 3:46 AM
ssastry merged a task: T239456: Server error 403 when previewing on Commons.
ssastry added a subscriber: Sebastian_Berlin-WMSE.
ssastry added a comment.Dec 3 2019, 3:50 AM

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)

ssastry added a comment.Dec 3 2019, 3:54 AM
In T239666#5707555, @Arlolra wrote:
In T239666#5707391, @Arlolra wrote:

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.

gerritbot added a comment.Dec 3 2019, 3:54 AM

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

ssastry added a comment.Dec 3 2019, 4:04 AM

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.

gerritbot added a comment.Dec 3 2019, 4:05 AM

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

gerritbot added a comment.Dec 3 2019, 4:07 AM

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

Maintenance_bot removed a project: Patch-For-Review.Dec 3 2019, 4:10 AM
Stashbot added a comment.Dec 3 2019, 4:15 AM

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)

Stashbot added a comment.Dec 3 2019, 4:19 AM

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.Dec 3 2019, 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 TranscriptDec 3 2019, 4:22 AM
ssastry mentioned this in T235179: Implement workarounds in RESTBase and Flow to hit Parsoid/PHP REST API endpoints without an oldid for titles containing ".".Dec 3 2019, 4:23 AM
ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.5; 2019-11-05).Dec 3 2019, 5:00 AM
Jdforrester-WMF added a comment.Dec 3 2019, 3:59 PM

Thank you, all!

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL