-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
params 'missing' due to rack bug #559
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
Comments
I think it would be good to have a test in Grape that breaks right now, then we can lock Rack >= 1.5.4 and merge the test. |
Please see #563 for a first cut at such a test |
Sorry, I'll leave you to close this if you think #563 covers this. |
I just spent half a day trying to track this bug down... finally figured it out, came here to report, and found this already here. Using the github#master version of rack (i.e. rack 1.6.0.alpha) seems to fix the problem by the way. |
@zuk rack 1.6.0.beta got released back in August. Your tip helped me a lot. |
Grape doesn't lock to a particular Rack version, so now that Rack 1.6.0 has shipped, this works. I committed the test with c72181d. |
Note that rack/rack#756 back-ported the fix into Rack 1.5.3. |
This commit in
rack
fixes an issue whereby all requests which use a temp file (all requests larger 128 kb?) fail to be properly parsed.The issue manifests itself in
grape
in the following way:Indeed this could have been the root cause of a few issues raised in the Google Group.
Not so much an issue from
grape
's perspective apart from it being the place where this error manifests itself. Definitely something to be aware of when runningRuby 1.9+
(commit comment suggests this doesn't affectRuby 1.8.x
)So more of an FYI in case anyone else gets bitten by this.
The issue is further complicated by the fact that running
rack/test
doesn't cross this codepath. Presumably because temp files are not used in tests??This issue is present in
rack == 1.5.2
, fixed inmaster
as of the time of this post, but not yet released (expected in1.5.4
I believe)The text was updated successfully, but these errors were encountered: