Return a "real" Symfony 404 response object instead of mimicking it

Super cool finding @Grevil! Really like this and I think it totally makes sense!

Anyway this might have a bunch of possible side effects, not at least for caching. Still I'm wondering why nobody else saw this before.

TBD:

• Manual testing
• Write tests
• Check if the text needs to be escaped, because its user-generated
• Evaluate if there might be other (negative) side-effects

Since there are no functional tests implemented for this module yet, I'd suggest we refer to the test issue, and create the tests there, after these fixes are committed to dev (preferably to a new SemVer Version), WITHOUT creating a new release until the tests are implemented there.

Referring to 📌 Create tests to test this modules functionality Active .

No need to escape the text, as "Response" strictly requires a string as the first parameter. Furthermore, it will simply get echoed on the page through "sendContent":

    /**
     * Sends content for the current web response.
     *
     * @return $this
     */
    public function sendContent(): static
    {
        echo $this->content;

        return $this;
    }

I am not 100% sure if code can be injected through "echo", but I found the following statement on the official php echo manual:

// Nicht-String-Ausdrücke werden in String umgewandelt, auch wenn
// declare(strict_types=1) verwendet wird

see https://www.php.net/manual/de/function.echo.php

So, it should be safe? I couldn't find any further information on this. Otherwise, there shouldn't be any negative side effect, but in the end, the maintainers should decide.

String doesn't mean it's (not) escaped, so I think if this comes from a variable and contains javascript for example, it will be output as-is.

So the answer is, we should have a look how Drupal handles such return values in core.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Util... might be what's needed.

Sounds good! I could find a few core implementations using "t()" or "FormattableMarkup" but they do not really fit our use case here, therefore I guess HTML::escape would definitly make sense here!

Please review once again!

Perfectly fine @Grevil! As written above this now needs tests and manual testing (did you try this already?) but in general, fine!! :)

(With tests I'd set this RTBC!)

Added appropriate tests.

Since there were no Functional nor FunctionalJS tests before, the newly added Test Class revealed the following schema errors:

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for search404.settings with the following errors: search404.settings:search404_use_customclue variable type is boolean but applied schema class is Drupal\Core\TypedData\Plugin\DataType\StringData, search404.settings:site_404 missing schema, search404.settings:search404_ignore_language missing schema

I will create a new issue for that.

See 🐛 Fix schema errors Fixed .

Thanks @Gravil.
The redirection is fine and the phpunit tests we can fix in 📌 Create tests to test this modules functionality Active as many unmerged issues are related to each others and need all fixed changes at one place for functional test cases.

@mitthukumawat thank you!! :) We're so happy to see activity here again! Are you planning to become co-maintainer and revive the maintainer activity here? Was very quiet recently.

@Anybody, I am working closely with Maintainer on this module.

Very well done @Grevil! Merged :)

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