Comments for rack's rack  

True, I just usually leave that up to dev and leave these out, but fair enough!

If you use rubygems, it will add the path for you. However if you are bundling rack, or using in any other case, you'll need to add lib to the load path. Otherwise the autoloads will not work.

Why does this need to be here at all?

Probably should include at least one test case for “multipart/related” uploads.

@rtomayko: got it, agreed, and +1

:-)

Dammit. Not sure how I missed that. `ack` FAIL or something …

@raggi said: “I’m interested to know why we need to coerce to a sepecific type – this seems very anti-ruby / duck typing.”

We’re not coercing to a certain type – #to_ary is an implicit cast, not an explicit cast like #to_a. Objects that aren’t Arrays (or extremely Array like) must not respond to it. The reason we check for arrayness is that we can’t calculate the Content-Length on anything that isn’t of fixed/known size. We can’t iterate over the body unless we know it’s already completely available and restartable.

@raggi: Duck typing is not a universal good. In widely-used public APIs where the desired object is one of String, Symbol, Array, Hash, or Proc, an implicit cast is generally preferred to a duck type. Duck typing has always been a powerful, but sharp tool, and if you’re not careful, you are absolutely in danger of cutting yourself with poor assumptions about how method names map to method behavior. The more widely-used the API, the greater the danger of introducing hard-to-debug issues. Implicit casts have their own share of danger, but they’re much, much easier to specify in an API.

As for to_ary returning self, anyone who does that deserves to have their code broken, unless self.kind_of?(Array). Doing a type check on to_ary’s return value would be total overkill, but in some cases (not here) might be justified, if for no other reason than to train people that to_ary should always return an Array.

I’m inclined to say that checking for to_ary and then calling each is a mistake. Consider the edge case where someone writes a class that could potentially be treated as say, both an Array and a Hash, but the each method implementation performs like a Hash and sends back a key and a value.

I’d probably argue that such a class is a bad idea to begin with, but I think you’ll find that there are no shortages of bad ideas that have managed to find their way into actual code.

It’s much more obvious to anyone reading the code that the behavior is correct if you call @body.to_ary.each { ... }.

Oh, and -2 to Array(@body), that’ll do inconsistent things with non-array datastructures, that’ll lead to hard to debug code paths, e.g.

run lambda { |env| [200, {}, DeferrableBody.new] }
...
Array(@body) #=> [#<DeferrableBody ...>]

And that breaches the rack spec now, even though the body handed back conformed to “responds to #each and yields string objects”. I could subclass under Array, but strictly speaking, it’s a streaming interface, and does not have an array like nature.

I think it shouldn’t be required, proxying #each is trivial, and then requires only a single implementation of an enumerable interface.

Using both just means people might be tempted to do stuff like:

  def to_ary
    self
  end

Or the like. I’m interested to know why we need to coerce to a sepecific type – this seems very anti-ruby / duck typing.

rtokayko: I'm not sure. In general I think it should be consistent: the respond_to? check should match the method you're using. I always thought that the purpose of the to_ary method was to indicate the object is effectively an Array. In this case I'd consider pairing @body.respond_to?(:to_ary) with @body.to_ary.each { ... }, but I might be more inclined to pair it with Array(body).each { … } instead.

As far as these changes I am fine with them. At the time, I thought it was the best course of action to update Rack::Lint to enforce either Content-Length or Transfer-Encoding is returned. The new implementation, where the handlers add these headers is better than ensuring every Rack app do it. My primary concern is the optimal headers are sent to the client.

I was actually thinking it might be nice to have some sort of HTTP proxy (HTTP::Lint?) that could sit in front of a server and inspect the HTTP headers, logging when something doesn’t match the specs. Or perhaps a suite of test cases that could be used to verify a server implements the spec properly.

I’ve had the same question myself. I’ve seen examples of both ways. Not actually calling #to_ary seems to be more prevalent. I think it’s a bit confusing, though and am happy to change it.

Also, what’s your feeling on these changes in general? I was hoping to run it by you since you were involved with the Thin Content-Length vs. chunked discussion that led to the previous behavior.

Should this line of code be checking if it responds to #to_ary, and then using #each inside the block? I guess most of the time something that can respond to #to_ary will be able to handle #each, but it’s not really gauranteed.

Was the intention within the block to do something like @body.to_ary.each { … } ?

Any estimate when this will be released in a new gem version? The multipart form bug is fairly problematic.

Way to soon. Was more drastic than I thought. Expect a ML thread on the issue soon.

Ouch! What happened?

Its a slippery slope, and its got razors at the bottom.

There’s nothing special about the name bytesize. Just because rails didn’t donkey patch it into String doesn’t mean that some other guy out there didn’t. Maybe he has a “DSL” where he defined bytesize globally to return the number of bytes a string would take when reencoded into utf-8 from jcs, or the number of bytes the string will take when encoded into a protocol header.

He’s going to have problems when he tries to port to 1.9, but those are his problems. If Rack starts silently using his crazy bytesize definition, his problems become yours.

Anyhow, just define Rack::size(s) as a module function, and use it, its only 2 more characters than the ruby1.9 form: size = body.size # 1.8 size = body.bytesize # 1.9 size = Rack::size body # compatibility wrapper

You can conditionally define it to return either size or bytesize depending on the ruby version, thus avoiding the overhead of continual respond_to? checks, AND having code that works perfectly with ruby 1.8 even when someone does have a weird bytesize method in their String.

Right. It’s different from, say, monkey patching #each in to 1.9. Adding a method like #bytesize that did not exist in 1.8 and that’s supported in 1.9 with the exact same semantics is probably one of the only places I’m okay with a library extending core objects.

I’m usually the first to decry monkey-patching, but I don’t have a problem with that suggestion. I’d say that’s perfectly acceptable. It doesn’t break the idiom for anyone else using it, and it simplifies a lot of repetitive code.