Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Problem when action request doesn't have Content-Length header
30-06-2013, 08:34 PM
Post: #1
Problem when action request doesn't have Content-Length header
If a control point sends an HTTP POST action request without a Content-Length header, ohNet rejects it with a 400 Bad Request response.

According to the UPnP Device Architecture 1.1 specification, devices must support this if the POST request body is sent with chunked encoding or if the connection is closed after the POST request body has been sent.

I discovered this when investigating a bug report from a MinimServer user for an interoperability failure when using a control point that sends POST requests without a Content-Length header (Simaudio MOON MiND).

I'm atttaching a patch to DviServerUpnp.cpp and DviServerUpnp.h to fix this. The added code is similar to that used in ProtocolUpnp.cpp and XmlFetcher.cpp. The patch has been tested by the user and confirmed as fixing the reported interoperability issue. I've also scanned the rest of the ohNet code to verify that there aren't any more instances of this problem.


Attached File(s)
.zip  contentlength.zip (Size: 1.26 KB / Downloads: 4)
Find all posts by this user
30-06-2013, 09:05 PM
Post: #2
RE: Problem when action request doesn't have Content-Length header
(30-06-2013 08:34 PM)simoncn Wrote:  If a control point sends an HTTP POST action request without a Content-Length header, ohNet rejects it with a 400 Bad Request response.

I'm attaching a patch to DviServerUpnp.cpp and DviServerUpnp.h to fix this.

Thanks for providing a fix as well as such a comprehensive report - its very much appreciated. I'll try to get this integrated tomorrow.
Find all posts by this user
01-07-2013, 04:04 PM
Post: #3
RE: Problem when action request doesn't have Content-Length header
(30-06-2013 09:05 PM)simonc Wrote:  
(30-06-2013 08:34 PM)simoncn Wrote:  If a control point sends an HTTP POST action request without a Content-Length header, ohNet rejects it with a 400 Bad Request response.

I'm attaching a patch to DviServerUpnp.cpp and DviServerUpnp.h to fix this.

Thanks for providing a fix as well as such a comprehensive report - its very much appreciated. I'll try to get this integrated tomorrow.

It might take me slightly longer to get this integrated. To aid native embedded clients with tight memory budgets, device stack code is often stricter than control point code about use of dynamic allocation. This means that patterns which are suitable for control point development aren't always appropriate on device side.

I'd like to find a way to implement this functionality without the dynamic per-action allocation. Its not immediately obvious how to do this so it might take me a couple of days to work through.
Find all posts by this user
01-07-2013, 08:24 PM
Post: #4
RE: Problem when action request doesn't have Content-Length header
(01-07-2013 04:04 PM)simonc Wrote:  It might take me slightly longer to get this integrated. To aid native embedded clients with tight memory budgets, device stack code is often stricter than control point code about use of dynamic allocation. This means that patterns which are suitable for control point development aren't always appropriate on device side.

I'd like to find a way to implement this functionality without the dynamic per-action allocation. Its not immediately obvious how to do this so it might take me a couple of days to work through.

I can see a simple improvement to my patch that would eliminate dynamic allocation for the normal case where Content-Length is specified.

Code:
TUint length = iHeaderContentLength.ContentLength();
if (length != 0) {
    iSoapRequest.Set(iReadBuffer->Read(length));
}
else {
    Bwh entity;
    if (iHeaderTransferEncoding.IsChunked()) {
        ReaderHttpChunked dechunker(*iReadBuffer);
        dechunker.Read();
        dechunker.TransferTo(entity);
    }
    else { // read until connection closed by control point
        try {
            for (;;) {
                Brn buf = iReadBuffer->Read(kMaxRequestBytes);
                entity.Grow(entity.Bytes() + kMaxRequestBytes);
                entity.Append(buf);
            }
        }
        catch (ReaderError&) {
            Brn snaffle = iReadBuffer->Snaffle();
            entity.Grow(entity.Bytes() + snaffle.Bytes());
            entity.Append(snaffle);
        }
    }
    iSoapRequest.Set(entity);
}

As you say, it is harder to see how to eliminate dynamic allocation for the other two cases (which should not occur with most control points).
Find all posts by this user
01-07-2013, 09:14 PM
Post: #5
RE: Problem when action request doesn't have Content-Length header
(01-07-2013 08:24 PM)simoncn Wrote:  I can see a simple improvement to my patch that would eliminate dynamic allocation for the normal case where Content-Length is specified.
....
As you say, it is harder to see how to eliminate dynamic allocation for the other two cases (which should not occur with most control points).

I'm going to try writing a dechunker that operates on the buffer already used by the server tomorrow. If that doesn't work, I'll go with the sort of approach you suggest.

As an aside, was the problem you saw with a CP using a chunked request? An HTTP 1.0 request that closed the connection to signify the request end doesn't make immediate sense - it would be unable to receive any response to its request (the invoked action).
Find all posts by this user
02-07-2013, 05:51 AM
Post: #6
RE: Problem when action request doesn't have Content-Length header
(01-07-2013 09:14 PM)simonc Wrote:  I'm going to try writing a dechunker that operates on the buffer already used by the server tomorrow. If that doesn't work, I'll go with the sort of approach you suggest.

As an aside, was the problem you saw with a CP using a chunked request? An HTTP 1.0 request that closed the connection to signify the request end doesn't make immediate sense - it would be unable to receive any response to its request (the invoked action).

An in-place dechunker would be an improvement, and should be possible.

Unfortunately the ohNet log messages don't give enough information to find out whether the request was chunked. I could find this out by building a diagnostic version of ohNet with extra logging and asking the user to rerun the scenario and send me the log data.

If a control point closes the outbound half of the socket after sending the request, I think it could still receive a response on the inbound half. I don't know whether the SOAP protocol allows this.
Find all posts by this user
02-07-2013, 07:53 AM (This post was last modified: 02-07-2013 08:47 AM by simonc.)
Post: #7
RE: Problem when action request doesn't have Content-Length header
(02-07-2013 05:51 AM)simoncn Wrote:  Unfortunately the ohNet log messages don't give enough information to find out whether the request was chunked. I could find this out by building a diagnostic version of ohNet with extra logging and asking the user to rerun the scenario and send me the log data.

If a control point closes the outbound half of the socket after sending the request, I think it could still receive a response on the inbound half. I don't know whether the SOAP protocol allows this.

Good point, I hadn't realised this was possible.

Please don't go to any effort confirming the exact nature of the original problem on my behalf. I had thought I could skip support for no Content-Length or chunking but I understand now that this is a valid CP behaviour. I'll add it in for the device stack server.
Find all posts by this user
02-07-2013, 06:11 PM (This post was last modified: 02-07-2013 07:45 PM by simoncn.)
Post: #8
RE: Problem when action request doesn't have Content-Length header
(02-07-2013 07:53 AM)simonc Wrote:  Good point, I hadn't realised this was possible.

Please don't go to any effort confirming the exact nature of the original problem on my behalf. I had thought I could skip support for no Content-Length or chunking but I understand now that this is a valid CP behaviour. I'll add it in for the device stack server.

I've done some more research into this, and it seems your original impression is correct. See this page for the HTTP rules. Specifically:

The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers. A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests. A server SHOULD read and forward a message-body on any request; if the request method does not include defined semantics for an entity-body, then the message-body SHOULD be ignored when handling the request.

and:

5.By the server closing the connection. (Closing the connection cannot be used to indicate the end of a request body, since that would leave no possibility for the server to send back a response.)

This means the following section from page 75 of the UPnP Device Architecture 1.1 specification is wrong:

CONTENT-LENGTH
REQUIRED if Origin Server does not close the session after sending the action invocation AND Origin Server does not send the
action invocation using chunked encoding.
PROHIBITED if Origin Server sends the action invocation using chunked encoding. OPTIONAL otherwise.
Field value specifies the length of the body in bytes. Integer.


The reference to "Origin Server" provides a clue that this wording has been incorrectly copied and pasted from another section of the document that specifies the rules for HTTP responses (pages 65, 77, 81 and 99).
Find all posts by this user
03-07-2013, 08:31 AM (This post was last modified: 03-07-2013 08:54 AM by simoncn.)
Post: #9
RE: Problem when action request doesn't have Content-Length header
(02-07-2013 06:11 PM)simoncn Wrote:  This means the following section from page 75 of the UPnP Device Architecture 1.1 specification is wrong:

CONTENT-LENGTH
REQUIRED if Origin Server does not close the session after sending the action invocation AND Origin Server does not send the
action invocation using chunked encoding.
PROHIBITED if Origin Server sends the action invocation using chunked encoding. OPTIONAL otherwise.
Field value specifies the length of the body in bytes. Integer.


The reference to "Origin Server" provides a clue that this wording has been incorrectly copied and pasted from another section of the document that specifies the rules for HTTP responses (pages 65, 77, 81 and 99).

This also affects the handling of event NOTIFY requests. It presumably implies that the corresponding paragraph on page 99 is also wrong, for the same reason (the HTTP 200 OK response would not be received). This would mean that the patch in this post was incorrect, as it causes ohNet devices to send HTTP 1.0 control points NOTIFY requests that don't include a Content-Length header. Similarly, this patch included code in EventUpnp.cpp to allow ohNet control points to receive these malformed NOTIFY requests.

Edit: The fix for this would be to include a Content-Length header in NOTIFY requests sent to HTTP 1.0 control points, but this would (rather ironically) require the device to use dynamic storage allocation to construct the NOTIFY request in a temporary buffer. Presumably it would not be necessary to do this buffer allocation on a per-request basis.
Find all posts by this user
04-07-2013, 08:12 AM (This post was last modified: 04-07-2013 08:13 AM by simonc.)
Post: #10
RE: Problem when action request doesn't have Content-Length header
(02-07-2013 06:11 PM)simoncn Wrote:  I've done some more research into this, and it seems your original impression is correct. See this page for the HTTP rules.

Thanks for doing this. I agree that it isn't valid for a HTTP/1.1 client to send a request without either a Content-Length: or Transfer-Encoding: Chunked header. I think it is still valid for a HTTP/1.0 client to connect to ohNet's 1.1 server and do this however (rfc2616 covers HTTP/1.1 only).

I think this means that your previous reports and suggested changes were valid. They could maybe be extended to confirm that only 1.0 requests assume read-request-until-disconnect. Unless you disagree, I'll add these tests.

Oh, and the original bug noted in this thread should be fixed and on github now Smile
Find all posts by this user


Forum Jump: