Castanet's issued HTTP requests are not compatible with Thin

[rsutphin]

Castanet issues HTTP requests using an absolute URI in the request line. Though this is legal per RFC 2616 section 5.1.2, Thin does not correctly handle requests of this form (see macournoyer/thin#8). Thin has benefits for integrated test deployments (in particular, its command line interface allows for SSL configuration, a seemingly unique feature among rack handlers), so it would be nice if Castanet would work with it.

[hannahwhy]

It looks like this be fixed by changing Net::HTTP#get invocations at e.g. https://github.com/NUBIC/castanet/blob/next/lib/castanet/service_ticket.rb#L144 to

h.get(uri.host, uri.path)

[rsutphin]

uri.path does not include the query string. I monkeypatched it in the test suite where I ran into this issue by duping the URI and clearing the scheme, host, and port. Then to_s on the dup gives you the string you need.

[hannahwhy]

Actually, before fixing this, I'd like to find out how many Web servers do not correctly handle this; as you mentioned, the behavior is compliant with RFC 2616, so I do not think it is erroneous. Additionally, this is a behavior of Net::HTTP, not Castanet.

If the error is in Thin, then I think effort is better spent there to make it handle this case.

[rsutphin]

Actually, before fixing this, I'd like to find out how many Web servers do not correctly handle this.

Thin is the only one I've tried specifically. I've read that Thin uses the same HTTP request parser as Mongrel, so it is possible that Mongrel would be similarly affected.

We know from experience that neither Webrick or Passenger/Apache have this problem.

If the error is in Thin, then I think effort is better spent there to make it handle this case.

Could be. I would note, however, that the aforelinked Thin issue (macournoyer/thin#8) has been open for more than two years. The response there was that they were not interested in fixing the problem "because web browsers never send absolute URLs."

A decision about whether it makes more sense to fix this in Castanet or Thin should also take into account the level of effort involved in each solution. And while Net::HTTP seems to work fine with absolute URIs, it's also worth noting that it refers to the Request-URI internally as path, suggesting that it was developed with server-relative URIs in mind.

[hannahwhy]

Well:

$ ruby -v
rubinius 2.0.0dev (1.8.7 028cb3df yyyy-mm-dd JI) [x86_64-apple-darwin10.8.0]
$ gem list mongrel

*** LOCAL GEMS ***

mongrel (1.1.5)
$ cat config.ru 
map '/foo' do
  run lambda { |env| [200, {'Content-Type' => 'text/plain'}, ['Hello world']] }
end

map '/bar' do
  run lambda { |env| [200, {'Content-Type' => 'text/plain'}, ['Goodbye world']] }
end
$ rackup -s mongrel

In another shell:

$ cat test-script 
GET http://localhost:9292/foo HTTP/1.1
Host: localhost:9292
Accept: */*

$ cat test-script | nc localhost 9292
HTTP/1.1 200 OK
Connection: close
Date: Mon, 16 Jan 2012 18:28:40 GMT
Content-Type: text/plain
Transfer-Encoding: chunked

b
Hello world
0

However, if you try this with Thin 1.3.1, you get

$ cat test-script | nc localhost 9292
HTTP/1.1 404 Not Found
X-Cascade: pass
Content-Type: text/plain
Transfer-Encoding: chunked
Connection: close
Server: thin 1.3.1 codename Triple Espresso

b
Not Found: 
0

as expected.

So I guess there's something else going on here. (Mongrel parser update, maybe?)

[rsutphin]

So I guess there's something else going on here. (Mongrel parser update, maybe?)

Entirely possible. The specific manifestation of the problem is that PATH_INFO in the rack environment is blank when you request an absolute URL under Thin. So it could also be a difference in interpretation of the data that the parser provides.

[hannahwhy]

I just tried the same Rackup file and test script with Unicorn and Puma (two more app servers that use Mongrel's parser), and they route requests correctly.

Given that you've got a working monkeypatch, and that Thin appears definitely in the minority, I'm going to see if I can fix this in Thin; let me know if this approach isn't acceptable. The "copy Mongrel 1.1.5's parser to Thin" approach doesn't fix the error, so it's probably something more subtle.

[rsutphin]

Given that you've got a working monkeypatch, and that Thin appears definitely in the minority, I'm going to see if I can fix this in Thin.

That sounds fine to me.

[hannahwhy]

While reading the documentation for Ruby's URI class I noticed something pretty cool:

http://www.ruby-doc.org/stdlib-1.9.3/libdoc/uri/rdoc/URI/HTTP.html#method-i-request_uri

That seems like exactly what we need for this, and it also seems to exist in 1.8.7:

http://www.ruby-doc.org/stdlib-1.8.7/libdoc/uri/rdoc/URI/HTTP.html#method-i-request_uri

Not sure how I missed this before.

[hannahwhy]

Castanet's CI build seems to have bit-rotted, so before I fix this issue I'm going to fix that. See #8.

[hannahwhy]

Can you give latest Castanet a shot and see if this issue is fixed?

[rsutphin]

Due to NUBIC/aker#23, aker-cas_cli's test suite won't run with castanet's current master. However, after patching castanet to disable certificate verification, the suite does pass with the thin monkeypatch removed. I'd call this fixed.