Enable AllowEncodedSlashes in mediawiki-quibble-apitests-vendor-php74-docker
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	abi_
	Oct 20 2022, 10:00 AM

Description

For testing variant conversion when using a title like MediaWiki:Privacy/crh-latn and sending the encoded value to the API (MediaWiki%3APrivacy%2Fcrh-latn) for testing returns in a 404 error page from Apache.

This was happening for me locally also and adding the following under the <VirtualHost> declaration fixed the issue: AllowEncodedSlashes NoDecode

Source: https://stackoverflow.com/a/9933890/903324

Details

	Subject	Repo	Branch	Lines +/-
	jjb: update Quibble jobs to add `AllowEncodedSlashes On`	integration/config	master	+31 -31
	dockerfiles: add `AllowEncodedSlashes On` to Quibble	integration/config	master	+57 -0

Customize query in gerrit

Event Timeline

abi_ created this task.Oct 20 2022, 10:00 AM

Restricted Application added a project: Continuous-Integration-Infrastructure. · View Herald TranscriptOct 20 2022, 10:00 AM

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

abi_ updated the task description. (Show Details)Oct 20 2022, 10:02 AM

daniel added a subscriber: awight.Oct 20 2022, 10:10 AM

Well done @abi_ you found the root cause I believe.

From Puppet, our MediaWiki has AllowEncodedSlashes On

$ git grep encoded_slashes
modules/mediawiki/manifests/web/vhost.pp:# [*encoded_slashes*] AllowEncodedSlashes apache httpd directive.
modules/mediawiki/manifests/web/vhost.pp:    Enum['On', 'Off', 'NoDecode'] $encoded_slashes = 'On',
modules/mediawiki/templates/apache/mediawiki-vhost.conf.erb:    AllowEncodedSlashes <%= @encoded_slashes %>

The CI images spawns an Apache driven by Supervisord. The configuration files are in integration/config.git under dockerfiles/quibble-buster/apache/. I think the setting should be added to the <VirtualHost> in 000-default.conf.

@abi_ do you have an example code which is broken at the moment? I would like to use it as a test case to confirm the change works ;)

Change 844937 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/config@master] dockerfiles: add `AllowEncodedSlashes On` to Quibble

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

gerritbot added a project: Patch-For-Review.Oct 20 2022, 10:51 AM

Thanks for getting on this so fast @hashar. The following test fails: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/841144/16/tests/api-testing/REST/Page.js

Here's the build that was failing: https://integration.wikimedia.org/ci/job/mediawiki-quibble-apitests-vendor-php74-docker/2192/console

Currently I've disabled those specific tests. Let me know when that change is deployed on the CI and I can update the patch to trigger those tests again.

Thanks, I have build the image locally. I will test it after lunch :)

Here is the test report, triggering the CI image locally requires a bunch of env variables to be passed which makes a longgggggg command line.

Running the API testing against https://gerrit.wikimedia.org/r/c/mediawiki/core/+/841144/16/tests/api-testing/REST/Page.js

With the currently deployed image (releng/quibble-buster-php74:1.4.6-s2):

$ docker run --rm -it --entrypoint=quibble-with-supervisord -v /home/hashar/projects:/srv/git:ro -e ZUUL_URL=https://gerrit.wikimedia.org/r -e ZUUL_PROJECT=mediawiki/core -e ZUUL_REF=refs/changes/44/841144/16 -e ZUUL_BRANCH=master docker-registry.wikimedia.org/releng/quibble-buster-php74:1.4.6-s2 --run=api-testing 
...
  309 passing (2m)
  68 pending
  2 failing

  1) Page Source
       GET /page/{title}/html
         Should perform variant conversion:
     AssertionError: expected '<!DOCTYPE HTML PUBLIC "-//IETF//DTD H…' to match /Гизлилик эсасы/
      at Context.<anonymous> (tests/api-testing/REST/Page.js:131:11)
      at processTicksAndRejections (internal/process/task_queues.js:95:5)

  2) Page Source
       GET /page/{title}/with_html
         Should perform variant conversion:
     AssertionError: expected '<!DOCTYPE HTML PUBLIC "-//IETF//DTD H…' to match /Гизлилик эсасы/
      at Context.<anonymous> (tests/api-testing/REST/Page.js:181:11)
      at processTicksAndRejections (internal/process/task_queues.js:95:5)

With the new image I have build locally:

310 passing (2m)
68 pending
1 failing

1) Page Source
     GET /page/{title}/with_html
       Should perform variant conversion:
   AssertionError: expected undefined to match /crh/
    at Context.<anonymous> (tests/api-testing/REST/Page.js:184:11)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

So that has addressed the prexisting errors and hit the same issue Daniel reported on the review. I am guessing that is solving it ;)

Change 844937 merged by jenkins-bot:

[integration/config@master] dockerfiles: add `AllowEncodedSlashes On` to Quibble

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

Maintenance_bot removed a project: Patch-For-Review.Oct 20 2022, 12:31 PM

Mentioned in SAL (#wikimedia-releng) [2022-10-20T12:40:43Z] <hashar> Building Quibble Docker images to add AllowEncodedSlashes On to Apache configuration | https://gerrit.wikimedia.org/r/c/integration/config/+/844937 | T321278

Change 844974 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/config@master] jjb: update Quibble jobs to add `AllowEncodedSlashes On`

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

gerritbot added a project: Patch-For-Review.Oct 20 2022, 1:03 PM

Mentioned in SAL (#wikimedia-releng) [2022-10-20T13:04:24Z] <hashar> Updating Jenkins jobs to add AllowEncodedSlashes On to Apache config https://gerrit.wikimedia.org/r/c/integration/config/+/844974 T321278

Thanks @hashar for quick resolution of this issue. Tests are now passing for this patch that uses encoded strings in the URL.

abi_ reassigned this task from abi_ to hashar.Oct 20 2022, 3:02 PM

daniel awarded a token.Oct 20 2022, 4:00 PM

@abi_ thank you for the verification!

Change 844974 merged by jenkins-bot: