flow-development-collection icon indicating copy to clipboard operation
flow-development-collection copied to clipboard

TASK: Move Neos Flow resources to Private resource and avoid publishi…

Open sorenmalling opened this issue 2 years ago • 5 comments

This pull request includes several changes related to the error handling and debugging functionality in the Neos.Flow package. Here's a summary of the changes made:

  1. In the DebugExceptionHandler.php file:

    • The path for including the CSS file (Exception.css) has been updated from ../../Resources/Public/Error/Exception.css to ../../Resources/Private/Error/Exception.css.
    • The path for including the JavaScript file (Exception.js) has been updated from ../../Resources/Public/Error/Exception.js to ../../Resources/Private/Error/Exception.js.
  2. In the Debugger.php file:

    • The path for including the CSS file (Debugger.css) has been updated from resource://Neos.Flow/Public/Error/Debugger.css to resource://Neos.Flow/Private/Error/Debugger.css.
  3. File and directory renaming:

    • The file Neos.Flow/Resources/Public/Error/Debugger.css has been renamed to Neos.Flow/Resources/Private/Error/Debugger.css.
    • The file Neos.Flow/Resources/Public/Error/Exception.css has been renamed to Neos.Flow/Resources/Private/Error/Exception.css.
    • The file Neos.Flow/Resources/Public/Error/Exception.js has been renamed to Neos.Flow/Resources/Private/Error/Exception.js.

These changes ensure that the error handling and debugging resources are loaded from the correct paths within the Neos.Flow package.

Resolves #3103

Review instructions

  • See that Neos.Flow does not create any folder in /Web/_Resources/Static
  • Throw an exception or print with Neos\Flow\var_dump and see that styling is still present.

Checklist

  • [x] Code follows the PSR-2 coding style
  • [x] Tests have been created, run and adjusted as needed
  • [x] The PR is created against the lowest maintained branch
  • [ ] Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • [ ] Reviewer - The first section explains the change briefly for change-logs
  • [ ] Reviewer - Breaking Changes are marked wit !!! and have upgrade-instructions

sorenmalling avatar Jun 25 '23 12:06 sorenmalling

hi ;) im not sure that this would really have a noticeable performance impact. And even if it did i would rather like to optimize our resource management than to leverage hacks like this.

What was your real reasoning behind this? ^^

edit, you can actually freeze the flow package if you want ^^

mhsdesign avatar Jul 05 '23 17:07 mhsdesign

What was your real reasoning behind this? ^^

First of

  • Have the filemonitor stop looking at files irrelevant. I can see that the development log loops over it again and again.

Second of

  • Have a empty public facing directory
  • Make it harder to determine the underlying framework of, say a API
  • Further on, fix this following issue https://github.com/neos/flow-development-collection/issues/3097

sorenmalling avatar Jul 07 '23 16:07 sorenmalling

I must say that im kind of against it. It would be pretty sad if we really need these kinds of micro optimizations imo

mhsdesign avatar Aug 15 '23 11:08 mhsdesign

It's no micro optimization.

And the point "Makes framework detection harder" is an absolut point, which security experts will generally approve. And disabling serving those files is server dependent and not easy. And absolutly not obvious. And as there is currently absolut no need to have them public they simply shoud not be.

You do not publish resources you dont use public.

fcool avatar Aug 15 '23 11:08 fcool

i was referring to

The few static resources of the Neos.Flow package makes the filemonitor listen to changes and try and publish them.

but i see your point. It will probably always possible somehow to detect if someone is using flow (and we are not even trying to hide it see Header Names, Cookie names, etc) but if you like this change i will not object

mhsdesign avatar Aug 15 '23 12:08 mhsdesign