Fast404 path checking is incompatibility with multilingual paths

The problem specifically occurs when (1) language negotiation is enabled and is configured to use a URL path prefix, and (2) Fast404 path checking is enabled in settings.php.

The attached patch addresses the issue by checking whether URL path prefix language negotiation is enabled, and whether the first component of the path matches a configured path prefix.

The patch in #2 fails for translated URLs.

This patch adds a check for configured URL aliases. This allows matching of translated paths.

I don't think this module is set up for automated testing. Putting this back in "needs review" status.

I found a new issue where a trailing slash after the path prefix (and nothing else) gets blocked by fast404 path checking. It should not be blocked.
Submitting new patch and interdiff.

Putting back in needs review: I don't think automated tests are working for this project.

I can confirm that when using the option $settings['fast404_path_check'] = TRUE;, language prefix paths return the fast_404 404 message without this patch. With the patch, the language prefix path renders as expected. Thanks Paul!

After reviewing a related issue #2743965: Fast 404 results in 404s on valid routes when using fast404_path_check, I think this patch needs a re-roll because it combines logic for language path prefixing as well as url aliases as seen in this bit below.

+    // Check whether the path matches a URL alias (exact matches only).
+    $sql = "SELECT pid FROM {url_alias} WHERE :path = alias";
+    $result = Database::getConnection()->query($sql, array(':path' => $path))->fetchField();
+    if ($result) {
+      return;
+    }

Here's a a new patch with the additional logic for path aliases removed. See this issue for path aliases, https://www.drupal.org/project/fast_404/issues/2743965

+++ b/src/Fast404.php
@@ -104,6 +104,33 @@ class Fast404 {
+    $lang_negotiation_url_info = \Drupal::config('language.negotiation')->get('url');

Quickly noticing that Drupal static is being used here a class context when dependency injection should be used instead.

Here is my attempt at DI the configfactory. I have both an alpha3 and dev version.

Alright, need alpha3 patch to test our code on hosted env. Will post summary of changes and dev patch shortly.

Thanks @robpowell - looking forward to reviewing the dev patch.

clean up setter method and put translation check back in Fast404::checkPath().

Alright, here we go, this works for us on alpha3. High level refactor points: it wasn't a matter of just adding DI. What we determined was Fast404 was coupled too closely to Fast404EventSubscriber. This means, that for any dependency to be injected it would have come from Fast404EventSubscriber and that is not maintainable. Instead, we implemented a factory, Fast404Factory/Fast404FactoryInterface that handles the instantiation of the Fast404 obj. Moving forward, if any code needs to create Fast404 object, it needs to include the Fast404Factory as a dependencies and call $this->fast404Factory->createInstance(). The createInstance() method is the centralize place to create Fast404 objects. Because we know that, we can make the Fast404 class unaware of the confg factory and load the language_negotiation conditionally saving the processing for only when translations are set.

Here is the dev patch.

Thanks, @robpowell. Attached is a quick code style cleanup for your dev patch.

I included the following patch in composer.json file and when I run "composer update drupal/fast_404" I am getting the below Error. Infact I even tried with the other versions of patch listed right at the top but I still get the same error. Please let me know how to apply this patch?

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2018-07-05/fast_404_2820923_24_dev.p...

@ismail08 I have the following in my composer.json

....
  "drupal/fast_404": "1.x",
....
"drupal/fast_404": {
            "#2820923-19: Incompatibility with multilingual paths": "https://www.drupal.org/files/issues/2018-07-05/fast_404_2820923_21_alpha3.patch",

Composer update should show a .reject somewhere in the module directory. This will tell you why the patch couldn't apply. It is my experience that this is when the version of the patch is out of sync of the local version of the dependency. My suspicion is that you are not on the 8.x-1.x branch of the module.

@bonus I was able to test dev with you updated changes. To get the site updated, I ran composer require "drupal/fast_404:1.x-dev" --update-with-dependencies and update my composer.json as follows:

"patches": {
  "drupal/fast_404": {
            "#2820923-24: Incompatibility with multilingual paths": "https://www.drupal.org/files/issues/2018-07-05/fast_404_2820923_24_dev.patch",
.....

I tested that fr-fr/ brought up a translated page. I also confirm that fr-fr/foo-bar brought up a quick 404. I think before this moves forward, it should get some tests and that is dependent on #2715585. I will put this is needs work and add the tag needs tests.

I've re-rolled latest patch against a version that includes patch from #2838359: Fast404 path checking is incompatible with Redirect module. I uploaded here, so I can test with project build.

Another try, did not include new files in my diff patch...also incorrect yaml indentation.

In the latest patches (including that from #27) Fast404Factory:: createInstance() it instantiates a new fast404 object with $fast404 = new Fast404($this->requestStack->getCurrentRequest());.

The patch from #2838359: Fast404 path checking is incompatible with Redirect module, adds $module_handler to the Fast404 constructor, so that it can check if redirect module is enabled or not. This next patch adds $module_handler to the Fast404Factory constructor so it can instantiate Fast404 with it.

Here's another patch.

I had to clean up a bit so it's not exactly a reroll but should apply on 8.x-2.x-dev and it's working for me.

PS: Missing tests.

I've re-rolled #27, which was the last working patch.

Fixed unit test failure from #39 with new construct() parameters.

Patch 40 works great! Thanks @juanolalla and everyone else above, especially @robpowell, @jasonawant @gg4 and @paulmckibbon

https://www.drupal.org/project/fast_404/releases/8.x-3.0-alpha1

Hey there,

The changes to fix this issue seem to have introduced a class not found error on https://git.drupalcode.org/project/fast_404/-/blob/8.x-3.x/src/EventSubs... (due to the removal of Fast404).

The bug has made its way into v3.0. I've attached a patch that works for me, not sure if this needs to be a new issue.

@Timmy_Cos Fyi, best to create a new issue, especially for fatal errors. Once an issue is closed/fixed, people often aren't looking at them.

New issue was opened here and fix was just merged:

#3374546: Fatal error : Class Fast404 not found