search

spray / spray-can

A low-overhead, high-performance, fully async HTTP 1.1 server and client library implemented entirely in Scala on top of Akka
http://spray.io
Apache License 2.0
153 stars 19 forks source link

HEAD Request Directives not processing when transparent-head-requests on #27

Open EdgeCaseBerg opened 9 years ago

EdgeCaseBerg commented 9 years ago

Hello!

I think I may have found a bug when dealing with HEAD requests to spray-can. The documentation states:

By default, spray-can handles HEAD-requests transparently by dispatching a GET-request to the handler and stripping of the result body. See the spray.can.server.transparent-head-requests setting for how to disable this behavior.

But it doesn't say that the head directives won't be processed as all when the setting is on. I've made an example repository with instructions on how to reproduce what I see as an error or undocumented "feature" here

The steps to reproduce are in the README file in the linked example. Is this the expected behavior? If so, where is it documented?

EdgeCaseBerg commented 9 years ago

Also, is this the right repository for this bug report? I assumed it was since the setting is spray.can.server and not something in spray routing

jrudolph commented 9 years ago

Nope, this is a very old legacy repository :)

This is also not a bug. As you quoted yourself, transparently-handled HEAD requests are dispatched to your application as GET requests. That's what this feature is all about. In the example, you need to react to GET requests to v0/healthcheck once you enable transparent head requests. So, it's exactly the head directive which prevents that the request is handled.

jrudolph commented 9 years ago

So, in summary, with spray.can.server.transparent-head-requests="on" you will never see any HEAD requests (they are transparent) but they will look to you like GET requests.

EdgeCaseBerg commented 9 years ago

I was surprised because while I expected the transparent GET requests, I didn't think that it would result in the head directives being ignored entirely for a route I only wanted to respond to HEAD requests. That wasn't totally clear to me from the documentation that it replaced the functionality, I just thought it added additional

EdgeCaseBerg commented 9 years ago

But isn't the tests passing no matter the setting (even with sealedRoutes) kind of misleading?

jrudolph commented 9 years ago

That's because tests built with the testkit are executed without spray-can and so transparent-head-request handling isn't available. I agree that this is unfortunate as it prevents proper testing of the behavior.

EdgeCaseBerg commented 9 years ago

I assume that there's no way currently to somehow bring that handling into scope in some way while doing tests? Short of actually running a spray-can instance and performing real HTTP requests against it.

It's not a big deal, since it does make sense now that you've explained that the transparent head requests replaces the head behavior with get. But I think it might be a nice todo if the documentation could be a bit more explicit about that. Maybe instead of saying: "handles HEAD-requests transparently by dispatching a GET-request", "handles HEAD-requests transparently by dispatching a GET-request instead of a HEAD request. It is not possible to unit test HEAD-only routes that do not have a corresponding GET route..."

Or some better-worded version of that. I'm sure it's not a typical use case to have a resource support HEAD and not GET so maybe it's not neccesary. But on the other hand, the inability to test that is, as you say, unfortunate.