grizzly
  1. grizzly
  2. GRIZZLY-1000

Requesting HEAD should not expect content

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.1
    • Component/s: http
    • Labels:
      None
    • Environment:

      OpenJDK 1.6

      Description

      When using Grizzly to request a resource via a HTTP HEAD method, the framework should not try to read a response. The fix should automatically set the HttpResponsePacketImpl.expectContent flag to false if the request method was HEAD. The right place to do this would seem to be in HttpClientFilter.onHttpHeaderParsed(...) where there is already a status code based check for making a similar determination.

      However, at this stage of processing, the response is uncorrelated to the request and there is therefore no way that I can see to determine whether the response packet being parsed originated from a HEAD request. Further, any attempt to add such a binding at this level would necessitate that the Grizzly Http framework become cognizant of pipeline ordering and I believe that it is completely stateless between request and response at present.

      This looked like it would be an easy fix and I was going to submit a patch, but there appears to be a design flaw at work here making any such fix non-trivial.

      I ran across this while trying to read the HEAD of a CouchDB document, but it can likely be reproduced against any endpoint that responds normally to HEAD requests.

        Activity

        Hide
        stellalaurenzo added a comment -

        Hmmm. That is how I was thinking to solve the problem, but when I traced through, the HttpResponsePacketImpl.getRequest() method returned null for the request. Has something changed between 2.0 and 2.0.1 so that the responses are correlated back to the requests?

        The reason I ask is that I am not sure how the http framework could do this without becoming stateful and still be able to handle pipelining. Do you know where in the code the response is being correlated to the request? I'm relying on the pipelining of requests and responses and would like to see what changes are in store for the next version.

        Show
        stellalaurenzo added a comment - Hmmm. That is how I was thinking to solve the problem, but when I traced through, the HttpResponsePacketImpl.getRequest() method returned null for the request. Has something changed between 2.0 and 2.0.1 so that the responses are correlated back to the requests? The reason I ask is that I am not sure how the http framework could do this without becoming stateful and still be able to handle pipelining. Do you know where in the code the response is being correlated to the request? I'm relying on the pipelining of requests and responses and would like to see what changes are in store for the next version.
        Hide
        Ryan Lubke added a comment -

        Yes, I've been working on implementing a Grizzly-based provider for the async http client library and ran into some similar issues with HEAD.

        The association occurs in HttpClientFilter.handleRead().

        Show
        Ryan Lubke added a comment - Yes, I've been working on implementing a Grizzly-based provider for the async http client library and ran into some similar issues with HEAD. The association occurs in HttpClientFilter.handleRead().
        Hide
        stellalaurenzo added a comment -

        This is what I was referring to by mentioning a potential design flaw.

        Creating a stateful binding in this fashion between the request coming through the write side of the chain to the response coming through the read side of the chain with a single valued attribute isn't quite right because it presumes that the next read will be the response for the last write, which presumes that callers issue a read and then wait for a response. This assumption pretty much breaks the ability to do request pipelining. Prior to this fix I don't think anything "cared" about the association from response -> request, so even if this association was being made, it didn't cause anything bad. As it stands now though, throwing a HEAD request into a pipeline will throw the whole thing off.

        A more correct solution to the overall problem, I think would be to maintain an attribute on the connection which is the LinkedList of pending requests. Then every time you get a response packet, pop one off the front and make the association.

        I haven't actually tried to break pipelining with how it is done now, but the current implementation is one of those HTTP anti-patterns I've come to recognize. Let me know if there is some other forum that I should bring this issue up in.

        Show
        stellalaurenzo added a comment - This is what I was referring to by mentioning a potential design flaw. Creating a stateful binding in this fashion between the request coming through the write side of the chain to the response coming through the read side of the chain with a single valued attribute isn't quite right because it presumes that the next read will be the response for the last write, which presumes that callers issue a read and then wait for a response. This assumption pretty much breaks the ability to do request pipelining. Prior to this fix I don't think anything "cared" about the association from response -> request, so even if this association was being made, it didn't cause anything bad. As it stands now though, throwing a HEAD request into a pipeline will throw the whole thing off. A more correct solution to the overall problem, I think would be to maintain an attribute on the connection which is the LinkedList of pending requests. Then every time you get a response packet, pop one off the front and make the association. I haven't actually tried to break pipelining with how it is done now, but the current implementation is one of those HTTP anti-patterns I've come to recognize. Let me know if there is some other forum that I should bring this issue up in.
        Hide
        Ryan Lubke added a comment -

        I think a new issue should be brought up for pipelining. It wasn't part of the original design here. Keep in mind, that a lot of the state-fulness in that method was there previously. We just linked the request/response sooner.

        Show
        Ryan Lubke added a comment - I think a new issue should be brought up for pipelining. It wasn't part of the original design here. Keep in mind, that a lot of the state-fulness in that method was there previously. We just linked the request/response sooner.
        Show
        stellalaurenzo added a comment - http://java.net/jira/browse/GRIZZLY-1002

          People

          • Assignee:
            Ryan Lubke
            Reporter:
            stellalaurenzo
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: