Router: Fix allocation of request buffer sent to application.

This fixes an issue reported by @Peter2121 on GitHub.

In nxt_router_prepare_msg() we create a buffer (nxt_unit_request_t *req)
that gets sent to an application process that contains details about a
client request.

The req structure comprises various members with the final member being
an array (specified as a flexible array member, with its actual length
denoted by the req->fields_count member) of nxt_unit_field_t's. These
structures specify the length and offset for the various request headers
name/value pairs which are stored after some request metadata that is
stored immediately after this array of structs as individual nul
terminated strings.

After this we have the body content data (if any). So it looks a little
like

  (gdb) x /64bs 0x7f38c976e060
  0x7f38c976e060: "\353\346\244\t\006"		<-- First nxt_unit_field_t
  0x7f38c976e066: ""
  0x7f38c976e067: ""
  0x7f38c976e068: "T\001"
  0x7f38c976e06b: ""
  0x7f38c976e06c: "Z\001"
  0x7f38c976e06f: ""
  ...
  0x7f38c976e170: "\362#\244\v$"		<-- Last nxt_unit_field_t
  0x7f38c976e176: ""
  0x7f38c976e177: ""
  0x7f38c976e178: "\342\002"
  0x7f38c976e17b: ""
  0x7f38c976e17c: "\352\002"
  0x7f38c976e17f: ""
  0x7f38c976e180: "POST"			<-- Start of request metadata
  0x7f38c976e185: "HTTP/1.1"
  0x7f38c976e18e: "unix:"
  0x7f38c976e194: "unix:/dev/shm/842.sock"
  0x7f38c976e1ab: ""
  0x7f38c976e1ac: "fedora"
  0x7f38c976e1b3: "/842.php"
  0x7f38c976e1bc: "HTTP_HOST"			<-- Start of header fields
  0x7f38c976e1c6: "fedora"
  0x7f38c976e1cd: "HTTP_X_FORWARDED_PROTO"
  0x7f38c976e1e4: "https"
  ...
  0x7f38c976e45a: "HTTP_COOKIE"
  0x7f38c976e466: "PHPSESSID=8apkg25r9s9vju3pi085i21eh4"
  0x7f38c976e48b: "public_form=sended"		<-- Body content

Well that's how things are supposed to look! When using Unix domain
sockets what we actually got looked like

  ...
  0x7f6141f3445a: "HTTP_COOKIE"
  0x7f6141f34466: "PHPSESSID=uo5b2nu9buijkc89jotbgmd60vpublic_form=sended"

Here, the body content (from a POST for example) has been appended
straight onto the end of the last header field value. In this case
corrupting the PHP session cookie.  The body content would still be
found by the application as its offset into this buffer is correct.

This problem was actually caused by a0327445 ("PHP: allowed to specify
URLs without a trailing '/'.") which added an extra item into this
request buffer specifying the port number that unit is listening on that
handled this request.

Unfortunately when I wrote that patch I didn't increase the size of this
request buffer to accommodate it.

When using normal TCP sockets we actually end up allocating more space
than required for this buffer, we track the end of this buffer up to
where the body content would go and so we have a few spare bytes between
the nul byte of the last field header value and the start of the body
content.

When using Unix domain sockets, they have no associated port number and
thus the port number has a length of 0 bytes, but we still write a '\0'
in there using up a byte that we didn't account for, this causes us to
loose the nul byte of the last header fields value causing the body data
to be appended to the last header field value.

The fix is simple, account for the local port length, we also add 1 to
it, this covers the nul byte, even if there is no port as with Unix
domain sockets.

Closes: <https://github.com/nginx/unit/issues/842>
Fixes: a0327445 ("PHP: allowed to specify URLs without a trailing '/'.")
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

1 files changed, 1 insertions, 0 deletions

diff --git a/src/nxt_router.c b/src/nxt_router.c
index 17f6c572..4637cc68 100644
--- a/src/nxt_router.c
+++ b/src/nxt_router.c
@@ -5208,6 +5208,7 @@ nxt_router_prepare_msg(nxt_task_t *task, nxt_http_request_t *r,
                + r->version.length + 1
                + r->remote->length + 1
                + r->local->length + 1
+               + nxt_sockaddr_port_length(r->local) + 1
                + r->server_name.length + 1
                + r->target.length + 1
                + (r->path->start != r->target.start ? r->path->length + 1 : 0);