Problem/Motivation

Given a multilingual environment with path prefixes as the language determination method, as well as path prefixes added for every language (also for default/english), 2 problems arise:

  • Going to admin/reports/status and clicking on the link "database update script" under database updates, results in a page not found (Drupal goes to /language_prefix/update.php
  • Go to /update.php manually and click continue. This goes well. However, when proceeding and clicking on "Apply pending updates" a page not found is displayed again (URL at this point: "/language_prefix/update.php/run"
    . Again, manually removing the language prefix results in the updates being executed properly

My site language settings:
My site language settings
My site language settings
My site language settings

When I try to update from RC3 to RC4...
[1] http://mysite/update.php - Overview page
[2] http://mysite/update.php/selection - Review updates page
[3] In this step I clicked the button for run the three or four updates and I get a 404 error:
http://mysite/es/update.php/run

To fix this, I had to edit the HTML code and remove language prefix ('/es' - My browser default language) from the Run updates button.

Drupal log (remember that I can finish my update editing the HTML code):
Drupal log

Attach a video:here

Proposed resolution

Apply the same URL generation for this link like others in the update process. Add tests.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#26 drupal-update_php_run_URL_is_generated_with_language_prefix_and_return_404_error-2616164-26-tests.patch3.34 KBagoradesign
#26 drupal-update_php_run_URL_is_generated_with_language_prefix_and_return_404_error-2616164-26-complete.patch5.48 KBagoradesign
#22 drupal-update_php_run_URL_is_generated_with_language_prefix_and_return_404_error-2616164-22.patch5.13 KBagoradesign
#19 drupal-update_php_run_URL_is_generated_with_language_prefix_and_return_404_error-2616164-19.patch2.15 KBagoradesign
#14 drupal-update_php_run_URL_is_generated_with_language_prefix_and_return_404_error-2616164-14.mp45.01 MBfacine
#10 drupal-update_php_run_URL_is_generated_with_language_prefix_and_return_404_error-2616164-10.patch1.1 KBfacine
#9 drupal-Updating_from_RC3_in_a_multilanguage_site_-_Error_404-004.png105.45 KBfacine
#8 drupal-Updating_from_RC3_in_a_multilanguage_site_-_Error_404-004.png105.45 KBfacine
drupal-Updating_from_RC3_in_a_multilanguage_site_-_Error_404-003.png43.39 KBfacine
drupal-Updating_from_RC3_in_a_multilanguage_site_-_Error_404-002.png79 KBfacine
drupal-Updating_from_RC3_in_a_multilanguage_site_-_Error_404-001.png18.54 KBfacine
#35 2616164-35.patch6.95 KBdawehner
#37 2616164-37.patch10.11 KBswentel
#40 2616164-40-interdiff.txt555 bytesswentel
#40 2616164-40.patch10.37 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

facine created an issue. See original summary.

facine’s picture

Issue summary: View changes
dawehner’s picture

facine’s picture

Title: Updating from RC3 in a multilanguage site - Error 404 » Updating from RC3 to RC4 or to 8.0.0. in a multilanguage site - Error 404
Version: 8.0.0-rc4 » 8.0.0
facine’s picture

Issue summary: View changes
cilefen’s picture

@facine Are you using Apache or some other server like Nginx?

facine’s picture

@cilefen: Apache/2.2.31

facine’s picture

facine’s picture

facine’s picture

Title: Updating from RC3 to RC4 or to 8.0.0. in a multilanguage site - Error 404 » /update.php/run URL is generated with language prefix and return 404 error
Version: 8.0.0 » 8.0.x-dev
Component: update.module » system.module
Status: Active » Needs review
FileSize
1.1 KB

This patch works for me.

facine’s picture

Title: /update.php/run URL is generated with language prefix and return 404 error » /update.php/run URL is generated with language prefix and returns 404 error
Gábor Hojtsy’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs issue summary update, +D8MI, +language-base

I cannot tell why/how is this fixing the issue honestly. It is not explained in the issue summary either. Anyway, would be good to have tests for so we can ensure it does not regress.

facine’s picture

Because this:

-      $url = (new Url('system.db_update', array('op' => 'run')))->setOption('base_url', $base_url);

Returns "/es/update.php/run" and this page display a 404 error.

\Drual\Url returns a path with language prefix.

And my solution is the same way that has used in the selection step:
http://cgit.drupalcode.org/drupal/tree/core/modules/system/src/Controlle...

facine’s picture

alvar0hurtad0’s picture

Hello all,

I've reproduced the issue, and the patch works fine.

Where is the best place to code the test?

Gábor Hojtsy’s picture

There is Drupal\system\Tests\Update\UpdatePathTestBase which is used in many tests to check for how the update.php process works. Either one of those should be extended or a new one added based on UpdatePathTestBase AFAIS.

Gábor Hojtsy’s picture

agoradesign’s picture

Coming from #2607156: update.php does not work properly with language prefixes - this patch here from this thread is an elegant way to solve the problem :) But as far as I can see, this only solves 50% of the problem. I'm pretty sure, that the link on the status report page is still wrong, as I had to patch the DbUpdateController as well as the system_requirements() function.

Please see my patch from the other thread here: https://www.drupal.org/files/issues/2607156-4.patch

agoradesign’s picture

Combined our two patches into one... only tests are missing now :)

PS: patch is so tiny, so I skip the interdiff...

facine’s picture

@agoradesign: Oh, thanks!

Someone can try to explain who to make the tests? I've never made one...

cilefen’s picture

In this case, you may be able to add something to Drupal\system\Tests\Update\UpdateScriptTest.

agoradesign’s picture

Added a test that simulates running an update on update.php in a multilingual environment, using a language-based path prefix

agoradesign’s picture

Status: Needs work » Needs review
cilefen’s picture

@agoradesign Can you upload a test-only patch along with the full patch so we can be sure it fails?

agoradesign’s picture

@cilefen: sure... I'm currently adding a few lines to the test case, inlcuding a new check, if the link to update.php on the status report page is correct (without language prefix)

Gábor Hojtsy’s picture

Issue tags: -Needs tests

Looks good to me. Once issue summary is updated, I can set it to RTBC :)

agoradesign’s picture

Issue summary: View changes

Like this?

Gábor Hojtsy’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

Touched up the proposed resolutions a bit. Looks good now!

alexpott’s picture

Priority: Major » Critical
Status: Reviewed & tested by the community » Needs review

I'm pretty sure this is a critical bug since if we have a critical bug that requires an upgrade path multilingual sites will not be able to apply it.

It's great that this adds test coverage.

+++ b/core/modules/system/src/Controller/DbUpdateController.php
@@ -370,15 +370,13 @@ protected function selection(Request $request) {
+        '#url' => Url::fromUri('base://run'),

+++ b/core/modules/system/system.install
@@ -638,7 +638,7 @@ function system_requirements($phase) {
+      $requirements['update']['description'] = t('Some modules have database schema updates to install. You should run the <a href=":update">database update script</a> immediately.', array(':update' => \Drupal::url('system.db_update', array(), array('language' => \Drupal::languageManager()->getLanguage(\Drupal\Core\Language\LanguageInterface::LANGCODE_NOT_APPLICABLE)))));

I'm a bit concerned about the routing assumptions this is making. It seems as though any use of the system.db_update route in a module will be dangerous because of this so should we able to add that to the routing information - opt out of path processing or something.

agoradesign’s picture

@alexpott: That's maybe worth a discussion, but this shouldn't be part of this issue/patch. As you can see, the use of the system.db_update route was not added by the patch, but is already part of the original code

alexpott’s picture

@agoradesign - yes I know that the patch does not add the route - but what the bug reveals is that we need a way of disabling path processing for specific routes. We have a very similar problem on #2583799: Install new module exposes bug in UnroutedUrlAssembler.buildLocalUrl() resulting in duplicated paths in URLs, such as /core/authorize.php/core/authorize.php

@dawehner suggested in IRC that we should be able to do something like $route->setOption('url_options' => ['path_processing' => FALSE]))

dawehner’s picture

@dawehner suggested in IRC that we should be able to do something like $route->setOption('url_options' => ['path_processing' => FALSE]))

Just to clarify this statement, we should add support for it.

dawehner’s picture

Here is a patch just for this general functionality.

jibran’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/src/Tests/Update/UpdateScriptTest.php
@@ -219,6 +220,57 @@ function testMaintenanceModeUpdateFunctionality() {
+  function testSuccessfulMultilingualUpdateFunctionality() {

Can we add this test to the patch as well?

swentel’s picture

Status: Needs work » Needs review
FileSize
10.11 KB

Merged in that test

xjm’s picture

Issue tags: +Triaged D8 critical

The D8 committers agreed that this is a critical issue.

Status: Needs review » Needs work

The last submitted patch, 37: 2616164-37.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review
FileSize
10.37 KB
555 bytes

Woops

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Nice trick! Also a backwards API addition, so should be possible in 8.0.x I hope :)

catch’s picture

Status: Reviewed & tested by the community » Fixed

Yes agreed that all looks fine for a patch release and nice fix.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

  • catch committed 8dc402f on 8.1.x
    Issue #2616164 by agoradesign, swentel, facine, dawehner: /update.php/...

  • catch committed e340b44 on 8.0.x
    Issue #2616164 by agoradesign, swentel, facine, dawehner: /update.php/...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Gábor Hojtsy’s picture

Issue tags: -sprint

Thanks all, yay!