terrylinooo / shieldon

Web Application Firewall (WAF) for PHP.
https://shieldon.io
MIT License
849 stars 98 forks source link

Extract $_SESSION to interface to comply with PSR-7 processing #19

Closed spotman closed 4 years ago

spotman commented 4 years ago

Dear Terry,

First of all, thank you for this awesome WAF!

I've found an issue with session processing in case of using https://github.com/php-pm/php-pm. Your code has direct access to $_SESSION super-global variable but projects based on the php-pm are fetching session from every request (e.q. PSR-7 message). Using $_SESSION in this case is useless because all requests will share the same session data. The best way to fix this is to extract session processing to separate interface, create default adapter for $_SESSION and [optional] adapters for each framework. This will allow developers to provide the correct session implementation and adopt their projects to php-pm, even without [optional] first-party framework-related adapters. $_SESSION adapter may be used by default so no BC break is expected.

AFAIK, using any super-globals like $_SERVER / $_GET / $_POST / $_COOKIE will break php-pm. So it seems that not only session processing needs to be rewritten but all super-global usages.

I would be happy to help you with this issue.

Regards, Denis.

terrylinooo commented 4 years ago

Thanks for your suggestion. I'll fix it tomorrow.

terrylinooo commented 4 years ago

After doing some research about this issue, I find that there is no suitable approach to make PHP native session compatible with PSR-7. I will use other ways instead of PHP native session in version 1.1.0

spotman commented 4 years ago

Yep, there is no standard way for session processing in PSR-7 / PSR-15 pipeline. Anyway, most frameworks rely on ServerRequestInterface to get session cookie value and fetch session data. As I said before, async processing requires all request data to be fetched from ServerRequestInterface and not from super-global variables. So the best way to achieve that is to remove access to all super-global variables and replace them with ServerRequestInterface instance + use default PSR-7 implementation + extract session management to separate interface / adapters. This is a huge change so I will be happy to help (I have experience in implementing PSR standards in several applications and libraries).

terrylinooo commented 4 years ago

PR is always welcome. When you have any idea, you can send your PR to development-1.10 branch.

spotman commented 4 years ago

OK. To clarify my vision before making any changes, I may suggest to:

1) write integration tests for PSR-15 processing 2) migrate to PSR-7 messages + adopt session processing 3) implement PSR-15 middlewares for firewall processing and control panel 4) add default PSR-15 pipeline for frameworks without support of PSR-15 5) add adapters for popular fraweworks (or use existing ones)

If this roadmap is OK, then I may start a PR. This will create BC breaks probably so it would be better to create a separate branch.

terrylinooo commented 4 years ago

Okay

terrylinooo commented 4 years ago

I have completed the PSR 7 impletation, and Shieldon 2.0 is on going. I will be available soon.