Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request attributes are being overridden #86

Open
KevinMarques opened this issue Dec 11, 2023 · 4 comments
Open

Request attributes are being overridden #86

KevinMarques opened this issue Dec 11, 2023 · 4 comments

Comments

@KevinMarques
Copy link

KevinMarques commented Dec 11, 2023

This PR #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.

@KevinMarques KevinMarques changed the title Request attributes are being overrided Request attributes are being overridden Dec 11, 2023
@mnapoli
Copy link
Member

mnapoli commented Dec 11, 2023

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 #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
Copy link
Author

KevinMarques commented Dec 11, 2023

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
Copy link
Contributor

juherr commented Dec 11, 2023

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
Copy link
Author

KevinMarques commented Dec 11, 2023

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 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants