Router icon indicating copy to clipboard operation
Router copied to clipboard

New feature : value control in unroute

Open guiled opened this issue 11 years ago • 12 comments

Is it possible to have a value control in Router\Http in the unroute ? In this lines https://github.com/hoaproject/Router/blob/af9b54379fd67c00bb419018e7eeb1482dba008e/Http.php#L596-L610 I think it could be possible to extract the regexp defining the value format and make a simple test on the concerned value given to unroute (edit: I change the source link)


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

guiled avatar Jan 11 '14 21:01 guiled

I assign @osaris on this PR :-).

Hywan avatar Sep 28 '14 18:09 Hywan

That make sense but my main concern is about performances ? Thoughts @hoaproject/hoackers ?

osaris avatar Oct 11 '14 14:10 osaris

It makes sense yes, but in which context? In a development environment maybe, in a production one it is better to let the error propagate and reach a 404 no (this is very stupid question)?

Hywan avatar Oct 11 '14 14:10 Hywan

If we allow to make “value validation“ into routes, it should be done in DEV and PROD environement, dev environment could be more verbose in exception message.

But i'm not sure it's to the router to do that. Your controller have already to check if the ID of the resource is exist, if the xxx value is valid, etc ...

However, a postValidation closure could be added to routes, not sure it's a good idea yet.

stephpy avatar Oct 13 '14 08:10 stephpy

What I meant by "value control" is to check the value with the regexp extracted from the route. For example : (?\d+) We should check if the value match with /\d+/ Why put that control in unroute where it seems to be the wrong place to do that? Because it is the only place where we can do it with the route definition... For the question DEV or PROD, I think we should have a parameter somewhere in the unroute process to enable it (it should be disabled by default) Then we could have something like _unroute($id, $pattern, $variable, $allowEmpty, $dataControl = false)

guiled avatar Oct 13 '14 15:10 guiled

I think @guiled is right when he says "it is the only place where can do it". You can't compare this check with the check of the existence of the ID as it's a lower level check here. We also need to keep in mind that this check is only for unroute so for data sent by the developer to the router (so data that must be clean), not for user input to the router. So it makes sense to only do this check in dev/test env and not production nope ?

osaris avatar Oct 13 '14 20:10 osaris

@osaris: :+1:. @guiled: Not sure about an argument to toggle (enable or disable) the validation. The user (of the library, so the developer) has written a regex. The URI must match the given the regex. So, from my point, we have to validate each time and not allow the user to disable this behavior, it has no meaning.

Thoughts?

Hywan avatar Nov 03 '14 10:11 Hywan

ping?

Hywan avatar Mar 27 '15 09:03 Hywan

ping :-)?

Hywan avatar Apr 13 '15 06:04 Hywan

Last ping?

Hywan avatar Jul 30 '15 06:07 Hywan

Validation on unroute can be usefull if a dev in a team "Damn this value xxx will match my regex of the death ?" and have a proper Exception "No dude, xxxx can not match /\d+/" (in dev mode ofc) But in prod mode this can be used for detect an hacking ? if we got "No dude, "; SELECT ..." can not match "/\d+/"

So i don't have any idea to code this :dancer:

thehawk970 avatar Jul 31 '15 13:07 thehawk970

Ok so new status: This issue is ready with a casual difficulty. This is not that hard to implement. If some one wants help, ping @osaris, @Metalaka, @vonglasow, @K-Phoen or me on IRC :-].

Hywan avatar Jul 31 '15 14:07 Hywan