Mongrel Rack handler fails to handle nil headers
Reported by Luke Redpath | May 20th, 2008 @ 06:13 AM | in 0.9.4
What is the problem?
When running a very simple, one-file Merb application, a POST to a controller method fails, with the log reporting an error. The controller method is very simple:
def create
Comment.create!(params[:comment].merge(:reference => params[:reference]))
store_credentials
redirect request.referer
end
The error is:
Read error: #<NoMethodError: undefined method `each' for nil:NilClass>
[truncated]/merb-core-0.9.3/lib/merb-core/rack/handler/mongrel.rb:77:in `process'
What is the cause?
My initial investigations tracked this down to the way the Mongrel Rack adapter attempts to process the headers from the tuple returned by Application#call. Here's the offending piece of code (merb 0.9.3):
# merb-core-0.9.3/lib/merb-core/rack/handler/mongrel.rb:77
headers.each { |k, vs|
vs.each { |v| ### => BANG!
response.header[k] = v
}
}
It turns out that one of the headers is nil, rather than a string or array which the above code expects. In my particular case, the Location header was nil.
What is the fix?
The obvious fix is to handle nil cases.
headers.each { |k, vs|
next unless vs ### => PHEW!
vs.each { |v|
response.header[k] = v
}
}
However, this could just be masking a lower-level problem. Is it valid to have a nil header? Should headers always have a value, or should headers with nil values be removed from the headers hash at an earlier point. What could be causing my app to have a nil Location header in the first place?
Unfortunately I do not have the answers to these questions. Over to you guys...
Comments and changes to this ticket
-
Michael Klishin (antares) May 20th, 2008 @ 06:18 AM
- → State changed from “new” to “open”
- → Milestone changed from “” to “0.9.4”
-
Luke Redpath May 20th, 2008 @ 06:27 AM
A few further points:
- After further testing, I'm convinced that the error in this case is due to the Location header being nil when it shouldn't be. The action in question is trying to redirect to request.referer. We know this worked with an older version of merb so perhaps the problem lies here?
- Despite this, I'm still wondering if the above code should be robust enough to handle nil headers, OR, produce a more meaningful error message that would lead to the discovery of an application bug or a bug elsewhere in merb more quickly.
Is there anything else I can provide you with?
-
Michael Klishin (antares) May 20th, 2008 @ 11:50 AM
Luke,
This should be enough for some initial investigation. I am no http expert but everything needed is in public access :)
I think it would be great if you post this to merb google group. Much more folks over there.
-
Michael Klishin (antares) June 2nd, 2008 @ 02:47 AM
- → Assigned user changed from “” to “Michael Klishin (antares)”
-
Michael D. Ivey (ivey) June 11th, 2008 @ 06:23 PM
What's the value of request.referer?
You need an escape hatch there for cases when referer is blank, anyway.
redirect request.referer || "/some/default"
-
Michael Klishin (antares) June 28th, 2008 @ 07:42 AM
- → Tag changed from “” to “controller dispatch failing handler medium merb-core mongrel”
Luke,
Because we cannot omit nil headers, what would be your suggestion? Log a warning maybe? HTTP_REFERER may be not set for some reasons like request using direct url. What do you think?
-
Michael Klishin (antares) June 28th, 2008 @ 03:56 PM
After some discussion it turns out that we either should add a warning or raise if body is nil. Rack does not assume you can return a body that is nil, it should be empty string then. And converting nil to string may mean hiding potential application problems.
Patch is coming soon.
-
Michael Klishin (antares) July 16th, 2008 @ 03:00 PM
- → State changed from “open” to “invalid”
It looks like neither nil body nor nil header values are valid in Rack.
If you have other information, leave a comment. Closing for now.
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
