certain requests return HTTP/1.0 400 Bad Request/Malformed request #12

Closed
danmcc opened this Issue Sep 3, 2011 · 3 comments

Projects

None yet

2 participants

@danmcc
danmcc commented Sep 3, 2011

It seems certain valid requests return HTTP/1.0 400 Bad Request with a "Malformed request" body, such as:

GET / HTTP/1.1
Host: www.exampleexample.com
Referer: www.exampleexample.com

We've been trying to narrow down the problem, but haven't had much success. Below is some code that reproduces the problem:

#!/usr/bin/perl

use warnings;
use strict;

use Feersum;
use IO::Socket::INET;

my $feersum_socket = IO::Socket::INET->new(
    LocalAddr => "0.0.0.0:8080",
    Proto     => 'tcp',
    Listen    => 1024,
    Blocking  => 0,
    ReuseAddr => 1,
);

my $evh = Feersum->new();
$evh->use_socket($feersum_socket);

$evh->request_handler(sub {
    my $r = shift;
    my $env = $r->env;

    my $w = $r->start_streaming(
      '404 Not Found',
      [ 'Content-Type'   => 'text/html',
        'Connection'     => 'close' ]
    );
    $w->close;
    return;
});

EV::run;

You can then do:

$ telnet localhost 8080
Trying 127.0.0.1...
Connected to localhost.localdomain.
Escape character is '^]'.
GET / HTTP/1.1
Host: www.exampleexample.com
Referer: www.exampleexample.com

HTTP/1.0 400 Bad Request
Content-Type: text/plain
Connection: close
Cache-Control: no-cache, no-store
Content-Length: 22

Malformed request.

We've traced this to this line in picohttpparser.c:

    if (*num_headers == max_headers) {
      *ret = -1;
    }

I thought this might be an issue with picohttpparser, so I modified the test case as below, but that seems to return fine (it prints the length of the string):

#include <stdio.h>
#include <string.h>
#include "picohttpparser.c"

void tests(int num)
{
  printf("1..%d\n", num);
}

void ok(int ok, const char* msg)
{
  static int testnum = 0;
  printf("%s %d - %s\n", ok ? "ok" : "not ok", ++testnum, msg);
}

int strrcmp(const char* s, size_t l, const char* t)
{
  return strlen(t) == l && memcmp(s, t, l) == 0;
}

int main(void)
{
  const char* method;
  size_t method_len;
  const char* path;
  size_t path_len;
  int minor_version;
  struct phr_header headers[4];
  size_t num_headers;

  const char* s = "GET / HTTP/1.1\r\nHost: www.exmapleexample.com\r\nReferer: www.exampleexample.com\r\n\r\n";

  num_headers = sizeof(headers) / sizeof(headers[0]);
  int i = phr_parse_request(s, strlen(s), &method, &method_len, &path,
           &path_len, &minor_version, headers,
           &num_headers, 0);
  printf("num: %d\n", i);

  return 0;
}
@stash
Owner
stash commented Sep 7, 2011

It seems there's a bug in the incremental mode of picohttpparser. I'd expect the "incremental 2" step to output the same as the "full" step, but it doesn't.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
#include "picohttpparser.c"

struct phr {
  const char* method;
  size_t method_len;
  const char* path;
  size_t path_len;
  int minor_version;
  struct phr_header headers[4];
  size_t num_headers;
};

int main(void)
{

  const char *s0 = "GET / HTTP/1.1\r\n";
  const char *s1 = "GET / HTTP/1.1\r\nHost: www.e.com\r\n";
  const char *s  = "GET / HTTP/1.1\r\nHost: www.e.com\r\nReferer: www.e.com\r\n\r\n";

  struct phr *p = (struct phr *)calloc(sizeof(struct phr), 1);
  p->num_headers = sizeof(p->headers) / sizeof(p->headers[0]);

  int last_len = 0;
  int i = phr_parse_request(s0, strlen(s0), &p->method, &p->method_len, &p->path,
           &p->path_len, &p->minor_version, p->headers,
           &p->num_headers, 0);
  printf("incremental 0 - i: %d, len %d\n", i, strlen(s0));

  last_len = strlen(s0);
  i = phr_parse_request(s1, strlen(s1), &p->method, &p->method_len, &p->path,
           &p->path_len, &p->minor_version, p->headers,
           &p->num_headers, last_len);
  printf("incremental 1 - i: %d, len %d\n", i, strlen(s1));

  last_len = strlen(s1);
  i = phr_parse_request(s, strlen(s), &p->method, &p->method_len, &p->path,
           &p->path_len, &p->minor_version, p->headers,
           &p->num_headers, last_len);
  printf("incremental 2 - i: %d, len %d\n", i, strlen(s));

  struct phr *p2 = (struct phr *)calloc(sizeof(struct phr), 1);
  p2->num_headers = sizeof(p2->headers) / sizeof(p2->headers[0]);
  i = phr_parse_request(s, strlen(s), &p2->method, &p2->method_len, &p2->path,
           &p2->path_len, &p2->minor_version, p2->headers,
           &p2->num_headers, 0);
  printf("full - i: %d, len %d\n", i, strlen(s));

  return 0;
}
@stash stash was assigned Sep 7, 2011
@stash
Owner
stash commented Sep 8, 2011

I think this is fixed in 8b5814d, which i just pushed to CPAN in 1.400

Can you please let me know if this doesn't fix the problem you're seeing?

@stash stash closed this Sep 8, 2011
@danmcc
danmcc commented Sep 8, 2011

Thanks so much. It does seem to have fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment