Plack::Request: die on superfluous arguments #325

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@avar
Contributor
avar commented Sep 5, 2012

I had some code that accidentally did
$plack_request->content_type($type) instead of
$plack_response->content_type($type), this just silently failed
instead of issuing a warning about the unknown argument.

I think Plack should die instead when you supply it with parameters
that it can't possibly do anything with. This is a partial hack to get
that going, this is just a proof of concept patch but a good start. In
order to fully execute on this we'd need to go through the whole plack
source and start dying on any unknown arguments.

This patch is best viewed with the --word-diff argument to git-show.

@avar avar Plack::Request: die on superfluous arguments
I had some code that accidentally did
$plack_request->content_type($type) instead of
$plack_response->content_type($type), this just silently failed
instead of issuing a warning about the unknown argument.

I think Plack should die instead when you supply it with parameters
that it can't possibly do anything with. This is a partial hack to get
that going, this is just a proof of concept patch but a good start. In
order to fully execute on this we'd need to go through the whole plack
source and start dying on any unknown arguments.

This patch is best viewed with the --word-diff argument to git-show.
98b787c
@miyagawa
Member
miyagawa commented Sep 5, 2012

I'm not sure if it's a good idea.

@avar
Contributor
avar commented Sep 5, 2012

Cool, that makes two of us. I really intended it more to start a discussion on the issue than as something to pull as-is.

I think it makes sense to at least warn about invalid use of the Plack API though, both to catch these kinds of issues and to make sure that there isn't some application in the wild that isn't going to be broken if Plack adds an nth argument to some function.

@miyagawa
Member

closing it for now.

@miyagawa miyagawa closed this Jan 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment