IBX-7044: Added dynamic route to settings submit form #72
Conversation
| @@ -112,6 +112,19 @@ public function updateAction(Request $request, UpdateView $view) | |||
| ]); | |||
| } | |||
|
|
|||
| if ($request->query->has('route')) { | |||
There was a problem hiding this comment.
Where this route param is populated?
Also, I guess return url being part of form make more sense.
There was a problem hiding this comment.
There was a problem hiding this comment.
I would suggest, at least, to change route name parameter as it really confusing. How abour return_route_name?
Not sure about it, but what about generating that whole URL in js, and just pass it here instead of route with params?
@dew326
There was a problem hiding this comment.
I think both solutions are valid. If you think it will be cleaner that way I'm ok with that change.
Gengar-i
force-pushed
the
ibx-7044-redirect-after-settings-update
branch
from
51617d9
to
6f36183
Compare
Gengar-i
force-pushed
the
ibx-7044-redirect-after-settings-update
branch
from
a072b77
to
6f36183
Compare
|
@alongosz Pointed possible danger while sending whole url to backend. So I stay with solution while passing route with parameters, which is safer solution. |
The point there was rather to sanitize input and/or attach path & query part to the current request. Now we might have different bug. What would happen if you pass existing route name different than expected, which has different parameters? |
I agree with Alongosz here, @Gengar-i. Route has to be checked before redirection - that it exists and can be created with those route parameters. You could fall back to a default redirect if they do not. |
|
Suggestion of solution.
Added dynamic route to settings submit form
I know passing url breaks security and it suppose to be done differently.
Checklist:
$ composer fix-cs)