PHP-DI / Slim-Bridge

PHP-DI integration with the Slim framework
http://php-di.org/doc/frameworks/slim.html
MIT License
176 stars 38 forks source link

Request attributes are being overridden #86

Open KevinMarques opened 9 months ago

KevinMarques commented 9 months ago

This PR https://github.com/PHP-DI/Slim-Bridge/pull/84 is causing the request attributes to be overriden.

As stated in the Slim official documentation:

With PSR-7 it is possible to inject objects/values into the request object for further processing. In your applications middleware often need to pass along information to your route closure and the way to do it is to add it to the request object via an attribute.

So you can for example have a middleware which sets in the request an attribute called "user" and have also a route with a "user" named argument, causing PHP-DI Slim bridge to override the original attribute value.

The Slim documented way to access route parameters is with $route->getArgument('id') function, so maybe the original issue here was the PHP-DI slim bridge documentation indicating a missuse of getAttribute function.

mnapoli commented 9 months ago

Thanks for the report!

So you can for example have a middleware which sets in the request an attribute called "user" and have also a route with a "user" named argument, causing PHP-DI Slim bridge to override the original attribute value.

In Slim (without this bridge) which one gets the priority?

We should probably respect the same priority?

The Slim documented way to access route parameters is with $route->getArgument('id') function, so maybe the original issue https://github.com/PHP-DI/Slim-Bridge/issues/76 was the PHP-DI slim bridge documentation indicating a missuse of getAttribute function.

I'm not 100% sure to understand what you meant in the last part?

Said another way: what do you think we should do?

KevinMarques commented 9 months ago

In Slim (without this bridge) which one gets the priority?

In Slim (and also with Slim-Bridge prior to v3.4) route parameters do not override attributes set in a middleware.

Edit: In Slim route parameters gets the priority.

I'm not 100% sure to understand what you meant in the last part?

What I meant there was that the change causing this unexpected behaviour was introduced in this PR, and that PR was based in this issue. I think the way @juherr and @reiniermybeats were trying to access route parameters was not correct (and thus the slim-bridge documentation they were relying on), because the Slim documented way to access route parameters is using getArgument function, not getAttribute.

Said another way: what do you think we should do?

I suggest simply reverting the changes introduced in v3.4.

juherr commented 9 months ago

The path argument management is not specified in the PSR-7 standard. Consequently, there is no built-in functionality for handling path arguments.

To avoid creating a strong dependency on the Slim framework in my controllers, I am reluctant to utilize its path argument management capabilities there and prefer if slim-bridge (or another middleware) manages it for me. Additionally, I fail to understand why php-di cannot offer an opinionated choice in this matter. However, I acknowledge that both approaches are valid options. Therefore, I would like to suggest the possibility of making this behavior configurable to accommodate different preferences and use cases.

@mnapoli Could you consider implementing this feature?

KevinMarques commented 9 months ago

I must update my answer. In Slim route parameters gets the priority. I checked again without Slim-Bridge and with Slim-Bridge 3.3 and 3.4 and only Slim-Bridge 3.3 gives priority to the attribute set in the middleware.

It's a bit fuzzy for me because I didn't find the Slim behaviour documented anywhere, but in any case Slim-Bridge from v3.4 is behaving correctly in line what Slim does.

As the PSR-7 standard does no specify how to handle this, it is up to you whether you want to allow this behavior to be configured or not.

Thanks anyway for shedding some light ;)