Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large request headers can be truncated #275

Closed
wch opened this issue May 29, 2020 · 2 comments · Fixed by #277
Closed

Large request headers can be truncated #275

wch opened this issue May 29, 2020 · 2 comments · Fixed by #277

Comments

@wch
Copy link
Collaborator

wch commented May 29, 2020

When request headers are large, they can get truncated.

Example:

library(httpuv)

# Create a web server app which returns the value of "test-header".
handle_headers <- function(req) {
  str(as.list(req))
  message("headers")
}
handle_request <- function(req) {

  # Store the headers in a global var named z, for inspection later.
  z <<- req$HEADERS

  list(
    status = 200L,
    headers = list('Content-Type' = 'text/plain'),
    # Use paste0("", ...) in case the header is NULL
    body = paste0("", req$HTTP_TEST_HEADER)
  )
}
s <- httpuv::startServer("0.0.0.0", 5000,
  list(
    onHeaders = function(req) handle_headers(req),
    call = function(req) handle_request(req)
  )
)


# Request a URL from the local web app
fetch <- function(url, headers = list()) {
  h <- curl::new_handle()
  do.call(curl::handle_setheaders, c(list(h), headers))

  # Make the request
  x <- NULL
  curl::curl_fetch_multi(
    url,
    handle = h,
    done = function(res) { x <<- res },
    fail = function(msg) {
      x <<- FALSE
      message("Failed: ", msg)
    }
  )
  # Pump both the curl and httpuv/later event loops until the download is finished.
  while (is.null(x)) {
    curl::multi_run(0)
    later::run_now()
  }

  x
}

# ===============================
# Create a request with a large header (81000 bytes)
long_string <- paste0(rep(".", 81000), collapse = "")
x <- fetch("http://127.0.0.1:5000",  list("test-header" = long_string))
content <- rawToChar(x$content)
nchar(content)
#> [1] 65380
identical(content, long_string)
#> [1] FALSE

The headers are stored in z. If you inspect them, there's a header named "" which has the remaining part of the header. Adding up the sizes of "test-header" and "", we get the original header size, which is 81000.

names(z)
#> [1] ""                "accept"          "accept-encoding" "host"           
#> [5] "test-header"     "user-agent"     
nchar(z)
#>                       accept accept-encoding            host     test-header 
#>        15620               3              13              14           65380 
#>   user-agent 
#>           46 

nchar(z[1]) + nchar(z[5])
#> 81000 

@aronatkins also provided a Python test app, which splits up the HTTP request into small TCP messages.

Details
#!/usr/bin/env python3
# Usage:
# ./test.py 127.0.0.1 5000

import json
import socket
import sys

host,port=(sys.argv[1], int(sys.argv[2]))
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.connect((host,port))

groups = []
for i in range(1000):
    groups.append("group-%d" % i)

value = json.dumps({'user':'admin','groups':groups})

message_template = """GET / HTTP/1.1
Host: %s:%s
Test-Header: %s

"""

message = message_template % (
    host,
    port,
    value,
)
message = message.encode()

sock = socket.socket()
sock.connect( (host, int(port)) )

sendsize=1
cur=0
while cur < len(message):
    sent = sock.send(message[cur:cur+sendsize])
    if sent == 0:
        raise RuntimeError("send sent nothing")
    cur = cur + sent

chunks = []
while True:
    chunk = sock.recv(1024)
    if chunk:
        chunks.append(chunk)
    else:
        break
print(''.join(chunks))
@wch
Copy link
Collaborator Author

wch commented May 29, 2020

The problem is that the HttpRequest::_on_header_value callback (which is invoked by the http parser) assumes that it will only be called once per value, but that's not necessarily true.:

httpuv/src/httprequest.cpp

Lines 253 to 254 in 7fbfd5e

_headers[_lastHeaderField] = value;
_lastHeaderField.clear();

@wch
Copy link
Collaborator Author

wch commented May 29, 2020

One more note: We should also consider increasing the HTTP_MAX_HEADER_SIZE for http-parser. We're currently using the default of 80kB, but there may be cases where a larger value is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant