HTTP "can service" query interface with 100 and 417
Joshua Peek
Rack::Cascade][1].
Cascade using 404s:
* Cons:
- Can't just return a return a 404
* Pros:
+ If the request is truly unserviceable by all apps in the cascade
with you will still get the 404 page from the last app.
+ Requires no special dance or modification to existing apps. Most
apps already return a 404 if they can't service the request so the
convention seems to fit.
I've been reviewing the HTTP RFC again and I've developed some of the
previous mentioned ideas a bit further. The goal is to standardize on
a more "httpish" API. The idea would be to make use of the 100 and 417
status codes.
100 Continue: The client SHOULD continue with its request. This
interim response is used to inform the client that the initial part of
the request has been received and has not yet been rejected by the
server. The client SHOULD continue by sending the remainder of the
request or, if the request has already been completed, ignore this
response. The server MUST send a final response after the request has
been completed.
417 Expectation Failed: The expectation given in an Expect request-
header field (see section 14.20) could not be met by this server, or,
if the server is a proxy, the server has unambiguous evidence that the
request could not be met by the next-hop server.
One possibility would be to use a continue expectation to first check
if the app could handle the request. If a 100 Continue where received,
go ahead and dispatch the request. Otherwise if a 417 was returned,
try the next app.
# cascade
env["Expect"] = "100-continue"
app = @apps.detect { |app| app.call(env)[0] == 100 }
env.delete("Expect")
app.call(env)
# foo app
if env["PATH_INFO"] = "/foo"
if env["Expect"] == "100-continue"
[100, ...]
else
# Do something
[200, ...]
end
else
if env["Expect"] == "100-continue"
[404, ...]
else
[417, ...]
end
end
* Pros:
+ The app is still completely standalone.
+ New API is supplements regular usage and allows to query an
applications capabilities.
* Cons:
- Apps would need to be modified to support the query API.
- Adds some complexity.
- Current Rack API doesn't allow multiple status codes to be
returned. If you try to return a 100, you automatically get a
"Connection: close" inserted and the connection is terminated
preventing you from returning the final response. This makes the query
API useless to the outside world and is only useful to other ruby
middleware (which is all we really need).
Since the whole 100 thing is kind of broken, we can still sort of work
around it. Option two would drop the query interface and no 100 would
ever be returned.
This is sort of still correct because the "server MAY omit a 100
(Continue) response if it has already received some or all of the
request body for the corresponding request." In all our cases, the
body has already been received. So we can just continue the request or
return a 417. NOTE: A 417 is only returned if the expectation is set,
otherwise the normal 404 error is returned. This allows the app to run
standalone.
# cascade
env["Expect"] = "100-continue"
apps.each do |app|
result = app.call(env)
break unless result[0] == 417
end
env.delete("Expect")
result
# foo app
if env["PATH_INFO"] = "/foo"
# Do something
[200, ...]
else
if env["Expect"] == "100-continue"
[404, ...]
else
[417, ...]
end
end
* Pros:
+ Still completely standalone.
* Cons:
- Apps would need to be modified to support the expectation
header.
- No fancy query interface.
[1]: http://groups.google.com/group/rack-devel/browse_thread/thread/ac228b58e6c36811/01588cbfa01d3d84
Michael Fellinger
This sounds best to me, the problem with 404 having double meaning
bites a lot of people, so I think that we can improve on it without
stirring too much trouble.
AFAIK, the 100 status should be reserved for asynchronous setups, and
the 417 description just sounds like Cascade kinda.
--
Michael Fellinger
CTO, The Rubyists, LLC
972-996-5199
James Tucker
I rejected 100 from the async specs due to the fact that people
complained at me that 100 is a valid client response. Frankly the
negative value solution that was settled on is the *ONLY* internal
messaging status code that simultaneously will never *ever* clobber
real HTTP codes, and does not require type coercion or non-immediates.
I recommended this last time, and I will continue to recommend it with
increasing vigour, having been down this road myself.
Joshua Peek
problems. But I don't see the "clobbering" real status codes problem.
You are only allowed to return a 417 if you ask for one (set an
expectation). AFAIK This is the intended purpose.
Sample patch to update file server to be expectation friendly.
def not_found
+ status = (env["Expect"] == "100-continue") ? 417 : 404
body = "File not found: #{@path_info}\n"
- [404, {"Content-Type" => "text/plain",
+ [status, {"Content-Type" => "text/plain",
"Content-Length" => body.size.to_s},
[body]]
end
James Tucker
On 30 Jun 2009, at 17:09, Joshua Peek wrote:
>
> So the 100 is probably out since it could cause the mentioned
> problems. But I don't see the "clobbering" real status codes problem.
> You are only allowed to return a 417 if you ask for one (set an
> expectation). AFAIK This is the intended purpose.
I used the same argument. 100 is never used for anything except
essentially what I was going to use it for, but the solution below
seems very dynamic.
Are you really sure that a -2 status code is too ugly to consider? Is
that why the suggestion is being ignored?
Michael Fellinger
>
>
> On 30 Jun 2009, at 17:09, Joshua Peek wrote:
>
>>
>> So the 100 is probably out since it could cause the mentioned
>> problems. But I don't see the "clobbering" real status codes problem.
>> You are only allowed to return a 417 if you ask for one (set an
>> expectation). AFAIK This is the intended purpose.
>
> I used the same argument. 100 is never used for anything except essentially
> what I was going to use it for, but the solution below seems very dynamic.
>
> Are you really sure that a -2 status code is too ugly to consider? Is that
> why the suggestion is being ignored?
I think, if we are going to use negative status codes for
communication between middleware, we could simply use a header
instead?
An alternative to that is to assign them to human-readable constants..
something like Rack::SKIP_CASCADE or Rack::ASYNCRON for official
middleware and Rack::Contrib::XXX for inofficial ones.
If we don't govern the use of negative status codes we are bound to
end up with collisions. Even the scheme i suggest still allows for
collisions.
Another way would be to use other kinds of objects (symbols?) in place
of status, the negative status codes are not valid according to the
Rack spec anyway (#to_i, but < 100) and aren't allowed to go past
Lint.
--
^ manveru
James Tucker
On 1 Jul 2009, at 10:40, Michael Fellinger wrote:
>
> On Wed, Jul 1, 2009 at 6:14 PM, James Tucker<jftu...@gmail.com>
> wrote:
>>
>>
>> On 30 Jun 2009, at 17:09, Joshua Peek wrote:
>>
>>>
>>> So the 100 is probably out since it could cause the mentioned
>>> problems. But I don't see the "clobbering" real status codes
>>> problem.
>>> You are only allowed to return a 417 if you ask for one (set an
>>> expectation). AFAIK This is the intended purpose.
>>
>> I used the same argument. 100 is never used for anything except
>> essentially
>> what I was going to use it for, but the solution below seems very
>> dynamic.
>>
>> Are you really sure that a -2 status code is too ugly to consider?
>> Is that
>> why the suggestion is being ignored?
>
> I think, if we are going to use negative status codes for
> communication between middleware, we could simply use a header
> instead?
True, might be a little OTT to scan for though, and headers may be too
unspecified externally to ever clear clobber. Ofc, we have some rack
namespaces we're using already in the env, so there's that too.
> An alternative to that is to assign them to human-readable constants..
> something like Rack::SKIP_CASCADE or Rack::ASYNCRON for official
> middleware and Rack::Contrib::XXX for inofficial ones.
Absolutely, I couldn't agree more. Rack::Contrib::XXX could actually
be a hash, and lazy alloc at runtime?
Rack::Contrib::SpecialStatusCodes[:my_special_code] # => generated
unique, consistent value.
In fact, one could use the object_id of the symbol, negative, on MRI.
> If we don't govern the use of negative status codes we are bound to
> end up with collisions. Even the scheme i suggest still allows for
> collisions.
Correct. A good point.
> Another way would be to use other kinds of objects (symbols?) in place
> of status, the negative status codes are not valid according to the
> Rack spec anyway (#to_i, but < 100) and aren't allowed to go past
> Lint.
I've considered this too. The reason I went for fixnums still is that
it seemed less likely to have edge cases, whilst using the signage
ensures being clear of the HTTP spec.
> --
> ^ manveru
Joshua Peek
> I used the same argument. 100 is never used for anything except essentially
> what I was going to use it for, but the solution below seems very dynamic.
>
> Are you really sure that a -2 status code is too ugly to consider? Is that
> why the suggestion is being ignored?
A negative status code sets up an invalid environment. "Status must be
>=100 seen as integer". This status code could never be sent out over
HTTP. If the intent is only to set a local flag for skipping and to
cause an error otherwise, then we are just abusing the status code
when raising an exception or throwing an error would be perfectly
fine. Using the appropriate status code allows us to provide the
"unserviceable" API even over HTTP.
--
Joshua Peek
James Tucker
A proxied environment, I see.
With regard to eventual termination, the status could be made valid
after the cascade.
I guess configurable ftw?
>
>
> --
> Joshua Peek
Eric Wong
> One possibility would be to use a continue expectation to first check
> if the app could handle the request. If a 100 Continue where received,
> go ahead and dispatch the request. Otherwise if a 417 was returned,
> try the next app.
>
> # cascade
> env["Expect"] = "100-continue"
> app = @apps.detect { |app| app.call(env)[0] == 100 }
> env.delete("Expect")
> app.call(env)
Unrelated to cascade, but I was inspired to make the following change to
Unicorn by the above. It requires modifying all the handlers/servers to
support this behavior, but I don't think it's too infeasible and not
many apps depend on 100-continue yet.
commit ec14a20474575e77a23b713ee8fcda1e71b1d018
Author: Eric Wong <normal...@yhbt.net>
Date: Wed Jul 1 13:32:33 2009 -0700
Move "Expect: 100-continue" handling to the app
This gives the app ability to deny clients with 417 instead of
blindly making the decision for the underlying application. Of
course, apps must be made aware of this.
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index a2893fd..281aa7d 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -444,7 +444,15 @@ module Unicorn
# once a client is accepted, it is processed in its entirety here
# in 3 easy steps: read request, call app, write app response
def process_client(app, client)
- HttpResponse.write(client, app.call(REQUEST.read(client)))
+ response = app.call(env = REQUEST.read(client))
+
+ if 100 == response.first.to_i
+ client.write(Const::EXPECT_100_RESPONSE)
+ env.delete(Const::HTTP_EXPECT)
+ response = app.call(env)
+ end
+
+ HttpResponse.write(client, response)
# if we get any error, try to write something back to the client
# assuming we haven't closed the socket, but don't get hung up
# if the socket is already closed or broken. We'll always ensure
diff --git a/lib/unicorn/app/inetd.rb b/lib/unicorn/app/inetd.rb
index 43a23eb..c3b8bbc 100644
--- a/lib/unicorn/app/inetd.rb
+++ b/lib/unicorn/app/inetd.rb
@@ -97,6 +97,10 @@ module Unicorn::App
end
def call(env)
+ expect = env[Unicorn::Const::HTTP_EXPECT] and
+ /\A100-continue\z/i =~ expect and
+ return [ 100, {} , [] ]
+
[ 200, { 'Content-Type' => 'application/octet-stream' },
CatBody.new(env, @cmd) ]
end
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index 3df9120..ad1e23f 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -89,9 +89,6 @@ module Unicorn
# returns a Rack environment if successful
def handle_body(socket)
PARAMS[Const::RACK_INPUT] = if (body = PARAMS.delete(:http_body))
- if 0 == body.size && /\A100-continue\z/i =~ PARAMS[Const::HTTP_EXPECT]
- socket.write(Const::EXPECT_100_RESPONSE)
- end
length = PARAMS[Const::CONTENT_LENGTH].to_i
if te = PARAMS[Const::HTTP_TRANSFER_ENCODING]
--
Eric Wong
Joshua Peek
> Unrelated to cascade, but I was inspired to make the following change to
> Unicorn by the above. It requires modifying all the handlers/servers to
> support this behavior, but I don't think it's too infeasible and not
> many apps depend on 100-continue yet.
@Eric Cool. Thats an interesting way to support 100 status codes. And
I don't think it interferers with anything I've purposed either since
returning a 100 before a 417 is not required.
@James Definitely think of it as middleware acting as a proxy. If an
expectation is already set before it hits the cascade, the cascade
should bubble up the 417 if no apps were matched. This allows cascades
to be stacked. I wouldn't recommend modifying the current
Rack::Cascade since it still has its own use. Maybe an
ExpectationCascade would be more appropriate. I could possibly draft
it up and commit it to contrib if you would like.
--
Joshua Peek
James Tucker
On 2 Jul 2009, at 05:20, Joshua Peek wrote:
>
> On Wed, Jul 1, 2009 at 6:29 PM, Eric Wong<normal...@yhbt.net>
> wrote:
>> Unrelated to cascade, but I was inspired to make the following
>> change to
>> Unicorn by the above. It requires modifying all the handlers/
>> servers to
>> support this behavior, but I don't think it's too infeasible and not
>> many apps depend on 100-continue yet.
>
> @James Definitely think of it as middleware acting as a proxy. If an
> expectation is already set before it hits the cascade, the cascade
> should bubble up the 417 if no apps were matched. This allows cascades
> to be stacked. I wouldn't recommend modifying the current
> Rack::Cascade since it still has its own use. Maybe an
> ExpectationCascade would be more appropriate. I could possibly draft
> it up and commit it to contrib if you would like.
Sounds good, I misread the intent as entirely in-application
previously, thus my desire to keep it out of the HTTP status code
range, however, this is a clearly different use case, and sounds like
it could be of definite use to some people. Go for it. :)
Joshua Peek
http://github.com/rack/rack-contrib/commit/cfee6eefe8291d0aa3c70a12b3b2cc350ec09beb
--
Joshua Peek