Correctly execute kernel events for initialize.php

[aschempp]

Yet another fix for the legacy initialize.php

In isotope/core#2064 we are modifying the session to store a state. Unfortunately, the session is not stored in our current approach, because that's done inside a kernel.response listener.

This hacky approach fixes this. It catches all the output of the entry point script, generates a response from it and sends it to the client.

TODO:

• Check if we need to handle HTTP/1.1 xxx status code headers

[qzminski]

We need to handle the headers sent via header() method as well because of custom code that potentially doesn't use the Contao's exceptions and/or Controller::redirect() or Controller::reload() methods.

[aschempp]

Unfortunately, tests seem to fail because of our special register_shutdown_function. I don't think we can test this component except in end-to-end tests 😞
We might wanna try that with https://github.com/symfony/panther but I guess that would only be for Contao 4.9+

[leofeyer]

Frankly, I am not sure whether all these changes are correct. The InitializeController is getting more and more complex and I do not even know which problems are being fixed or if the changes are really fixing them.

The InitializeController was meant to provide a thin BC layer for legacy entry points which still include the system/initialize.php file. However, the idea was not to blow up the controller until it works perfectly with legacy entry points but to refactor the legacy entry points and eventually get rid of them. 🤷‍♂

Contao 3 has reached its ultimate EOL, therefore maybe it is time to release an Isotope version that only supports Contao 4?

[aschempp]

Frankly, I am not sure whether all these changes are correct.

I can explain it to you, or we can just assume if they work in Isotope they should be correct 🤷‍♂
Btw. @qzminski is supposed to validate this as well, he as extensions with custom entry points too.

therefore maybe it is time to release an Isotope version that only supports Contao 4?

yes it is, but it's still a long way to go…

[leofeyer]

Can you elaborate on the ob_start() and register_shutdown_function() usage? Why is it necessary to use such a hacky workaround?

Btw. @qzminski is supposed to validate this as well, he as extensions with custom entry points too.

What is the status on this one?

[aschempp]

The problem is that we're currently not executing the kernel.response events. Because of that, the Contao session bags are not saved to the session, so any legacy code working with the legacy session will loose its data.

[leofeyer]

This does not explain why we have to use register_shutdown_function() though, does it?

[qzminski]

What is the status on this one?

I got an e-mail today that one of my legacy scripts that uses old initialize.php is being triggered as a download since version 4.9 but I will further investigate the case in the next days.

[aschempp]

This does not explain why we have to use register_shutdown_function() though, does it?

well it does, if you know how it works inside-out 😂

1. the entry point is executed
2. the entry point loads initialize.php
3. initialize.php creates a fake route to run kernel.request events
4. the fake route render the InitializeController that returns a specific response
5. if the initialize.php receives the correct response, it does nothing more, otherwise it exists (e.g. a kernel.request replaced the response)
6. now the entry point script continues
7. the entry point script runs whatever it has to run
8. after the entry point script, the kernel.response event should be run, to save session etc. Obviously, running after the entry point script is rather tricky, that's why we use register_shutdown_function

[leofeyer]

8. Obviously, running after the entry point script is rather tricky, that's why we use register_shutdown_function

That makes sense. 👍

Then I guess we'll wait for @qzminski's investigations and merge this PR afterwards. The unit tests need to be fixed though. 🙈

[aschempp]

I‘m pretty sure we can‘t fix the unit tests. We might add end-to-end tests for 4.9 🤷‍♂️

[leofeyer]

Not fixing it is not an option obviously. We at least have to remove the failing tests to fix the CI chain.

[aschempp]

Absolutely, thats what I mean. As this is for 4.4 I would suggest to just remove the tests and then try to add e2e tests when merged into 4.9

[aschempp]

@qzminski found an issue with the Symfony response related to symfony/symfony#35709

Since we are now executing the kernel.response event ourselves, we don't want to run these listeners for the InitializeControllerResponse.

@qzminski please test this change in your local setup.

[fritzmg]

With these changes I get an empty response with custom entry points - though only within certain server environments. For example, the treepicker.php entry point of codefog/contao-widget_tree_picker returns an empty response on Hostingwerk (Apache profile). However, locally on my system (nginx) the response content is returned as expected.

[fritzmg]

@qzminski have you noticed anything like that yet?

[qzminski]

@fritzmg kinda yes but it was fixed by @aschempp in ff09091, so the origin of your problem must be totally different. I don't know what could be wrong in your case, though 🤷🏻‍♂️

[fritzmg]

Hm, ok. I'll make a test installation on Hostingwerk, once Contao 4.4.49 is released.

[qzminski]

By the way, shouldn't you use the new DCA picker providers feature instead of this legacy extension? Or is it tied down to some of mine open source extensions? 😅

[fritzmg]

By the way, shouldn't you use the new DCA picker providers feature instead of this legacy extension? Or is it tied down to some of mine open source extensions? 😅

Well, it's an installation that still uses codefog/contao-news_categories in Version 2.8 (and can't be easily updated to ^3.2 due to a lot of customization regarding the news categories). I take it codefog/contao-news_categories >=3.0 does not use codefog/contao-widget_tree_picker anymore?

[qzminski]

Yep exactly, the new picker was introduced in v3.0 of codefog/contao-news_categories.

[fritzmg]

Ok thank you, good to know :)

[aschempp]

See #1728