I would love to fix it but honestly don't know how :( PresenterComponentReflection::convertType() correctly processes the parameter as zero.

Actual result is IMHO fine.

IMO is't a bug.

I think it's a bug too. "int" represents all integers including negative ones and zero and I don't see a reason why a 0 integer should be treated differently.

This ?id=0 can be achieved in this way:

public function actionEdit(int $id = NULL)

actionEdit(int $id = NULL) means something different to me, it means that the parameter $id is optional and nullable. But actionEdit(int $id) means that $id is required and ranges from PHP_INT_MIN to PHP_INT_MAX and it cannot be NULL.

I too think it's a bug unless the action is with default value (actionFoo(int $id = 0)). 0 is a valid value.

So what it should do when a=0 is not in URL?

So what it should do when a=0 is not in URL?

This question is about matching URLs and this PR is about constructing them.

But I'd say in this case the result of $this->getParameter('id') is null and null cannot be passed to int $id, so it's basically a type mismatch.

When a is not present, null should be passed to method call, an php exception will be triggered and handled by the app's error presenter.

i.e. it should throw 500 error? Ohh…

It means that the router is misconfigured/implemented poorly so I'm fine
with 500 too.

On Thursday, 21 January 2016, David Grudl notifications@github.com wrote:

i.e. it should throw 500 error? Ohh…

—
Reply to this email directly or view it on GitHub
#112 (comment).

Ondřej Mirtes

It means that the router is misconfigured/implemented poorly so I'm fine with 500 too.

👍

It is not about router. We are talking about ?a=0 in query.

And 500 means server error. Server error due to the removal of a single parameter from URL? Boys! Do not be silly :-)

As I proposed couple of years ago, missing required parameter (or wrong type, e.g. abc when int is required) should result in BadRequestException#404. Note that any such change would probably require new major version.

I want the HTTP request to give me an integer. If there's no integer in the request, I have to respond with an error. What kind of error is a subject to discuss.

I think this layer of request processing should be stopped at the router level. If an incorrect type is received by a presenter (involving some lossless type casting), something is very wrong and I would like this situation to result in a new error log line. So I'm voting for a 500 error.

But if Nette wants to be more loose in these situations, I can accept throwing 404 too.

@dg: The issue was about generated URLs, not their handling.
If you require a parameter in the action and it happens to be missing in the request, it is obviously not OK. What @JanTvrdik proposes makes sense in such case.

Also I see a huge difference now that we have PHP 7 with scalar types. In the old times we often used default value (e.g. actionFoo($id = 0)) just to express the default type, but it wasn't valid signature as the parameter is required. With PHP 7 we no longer need to use this hack as we can use proper typehints (e.g. actionFoo(int $id)).

Possibly related (5 years old) forum thread https://forum.nette.org/cs/6819-validace-parametru-v-presenterech but due to it's age lot's of information and ideas may be outdated

i.e. it should throw 500 error? Ohh…

It doesn't mean, that the resource does not exist. You didn't fullfill the contract, server failed to do his job. That's all.

I'm probably ok with 400. But 404 is non-sence.

Btw, you probably look for 422: http://www.restpatterns.org/HTTP_Status_Codes/422_-_Unprocessable_Entity

Can we go back to the original issue of constructing URL with zero integer as a parameter and create a new thread for support for scalar typehints in presenters and how to solve receiving requests with them? Thank you.

@JanTvrdik As I proposed couple of years ago, missing required parameter should result in BadRequestException#404

This is obviously a good idea, but the giant BC break.
But it would be possible with scalar typehints because they are new (so no BC break), just inconsistency.

@ondrejmirtes I want the HTTP request to give me an integer.

It does :-) It gives you 0. Btw, whether it is, number of page or id, you have to validate it.

… I have to respond with an error. What kind of error is a subject to discuss.

Absolutely not. There's nothing to discuss, it just needs to be BadRequestException. (ie. 4xx, imho 404 is fine)

I think this layer of request processing should be stopped at the router level.

How can you define in router, that the query must contains a parameter a? And numeric in addition.

@Majkl578 The issue was about generated URLs, not their handling.

Generating and matching are very close. We cannot discuss about first one and aviod discussion about second one.

It does :-) It gives you 0. Whether it is, number of page or id, you have to validate it.

If there's a missing parameter in the request, I don't want a random integer. Zero has the same weight as any other integer.

How can you define in router, that the query must contains a parameter a? And numeric.

/admin/product/<presenter>/<action>[/<id [1-9][0-9]*>] - this is how we define positive integers in id parameter in masks in Nette\Application\Routers\Route. Any integer can be defined as [0-9]+.

@ondrejmirtes Can we go back to the original issue of constructing URL with zero integer as a parameter and create a new thread for support for scalar typehints in presenters and how to solve receiving requests with them? Thank you.

No, it is complex problem. You have to take into account the compatibility. What happens when someone will upgrade PHP 5 renderDetail($id = 0) to PHP 7 renderDetail(int $id). In old links there is no id=0. Etc.

(ad router: I mean query part of URL).

renderDetail($id = 0) and renderDetail(int $id) mean something different, the first one has default value and the second one does not. Correct refactoring to PHP 7 code is from renderDetail($id = 0) to renderDetail(int $id = 0).

Correct refactoring to PHP 7 code is from renderDetail($id = 0) to renderDetail(int $id = 0).

👍

@pilec: 422 is not valid for this case, because request URI is not an entity (entity = http body).

1. we can responds with 404 when parameter with scalar typehint and without default value is missing

• no BC break (or tiny)
• solves this issue
• makes inconsistency with typehint array, with parameters without typehints and persistent parameters

2. we can responds with 404 when any parameter without default value is missing

• giant BC break
• solves this issue
• all parameters wil be handled the same way

3. we can pass default values to parameters with scalar typehits and without default values (current behavior)

• no BC break
• solving this issue will create inconsistency how Nette generates URL
• all parameters are handled the same way

I think that 2) is impossible and 1) vs 3) is about consistency (ie. a about different handling of array $arr vs. int $id etc).

IMHO 1) is best way.

IMHO 1) is best way.

👍 , still, I'm more for 400.

400 is perfectly ok, I'll try to find out how it affects Google. For example, 410 is big problem, because Google will ignore this URL, although it becomes exist in future. (And maybe that's not true anymore.)

400 is good from the point of view of the HTTP spec, but it will likely cause people having to write a new latte template or change their code in the ErrorPresenter.

I think that the best choice for Nette 3 is (1) + change array typehint behavior to match other typehints (BC break) + possibly implement support for class typehints (e.g ˙show (Article $article)˙)

Class type hints are IMHO not needed, because object is created in router, which is tightly coupled with presenter.

Now I realized that they are already suppoted!

I forgot to mention persistent parameters - so 1) makes inconsistency between typehinted parameters and persistent parameters, because properties in PHP cannot have typehints. It is possible to use annotation @var, but it will be BC break.

@JanTvrdik change array typehint behavior to match other typehints (BC break)

It is tricky, because it is not possible to have empty array in URL. In fact, array is consistent with both solutions.

Random thoughts:

• We could make array $list mean "at least 1 item" and make array $list = [] mean "any number of items".
• You cannot have empty array in URL but you can have empty array in application request.

Cool, thanks! 👍

Great! Thank you!

Looks great 👍