Permalink
Browse files

Correctly handle 304 responses.

Also handle a 3-digit string message properly.
  • Loading branch information...
1 parent 7d82876 commit 61d015b2f1d40894bf70d8b69d2d84d55493b2cd @stash committed Oct 6, 2010
Showing with 113 additions and 13 deletions.
  1. +36 −12 Feersum.xs
  2. +69 −0 t/10-respond-304.t
  3. +8 −1 t/Utils.pm
View
@@ -117,9 +117,10 @@ struct feer_conn {
U16 in_callback;
U16 responding;
U16 receiving;
- U16 _reservedflags:14;
+ U16 _reservedflags:13;
U16 is_http11:1;
U16 poll_write_cb_is_io_handle:1;
+ U16 auto_cl:1;
};
typedef struct feer_conn feer_conn_handle; // for typemap
@@ -156,6 +157,7 @@ static void respond_with_server_error(struct feer_conn *c, const char *msg, STRL
static STRLEN add_sv_to_wbuf (struct feer_conn *c, SV *sv);
static STRLEN add_const_to_wbuf (struct feer_conn *c, const char const *str, size_t str_len);
+#define add_crlf_to_wbuf(c) add_const_to_wbuf(c,CRLF,2)
static void finish_wbuf (struct feer_conn *c);
static void add_chunk_sv_to_wbuf (struct feer_conn *c, SV *sv);
static void add_placeholder_to_wbuf (struct feer_conn *c, SV **sv, struct iovec **iov_ref);
@@ -318,7 +320,7 @@ add_chunk_sv_to_wbuf(struct feer_conn *c, SV *sv)
struct iovec *chunk_iov;
add_placeholder_to_wbuf(c, &chunk, &chunk_iov);
STRLEN cur = add_sv_to_wbuf(c, sv);
- add_const_to_wbuf(c, CRLF, 2);
+ add_crlf_to_wbuf(c);
sv_setpvf(chunk, "%x" CRLF, cur);
update_wbuf_placeholder(c, chunk, chunk_iov);
}
@@ -1455,21 +1457,38 @@ feersum_start_response (pTHX_ struct feer_conn *c, SV *message, AV *headers,
}
I32 avl = av_len(headers);
- if (avl < 0 || (avl % 2 != 1)) {
- croak("expected even-length array");
+ if (avl+1 % 2 == 1) {
+ croak("expected even-length array, got %d", avl+1);
}
// int or 3 chars? use a stock message
- if (SvIOK(message) || (SvPOK(message) && SvCUR(message) == 3)) {
- int code = SvIV(message);
+ UV code = 0;
+ if (SvIOK(message))
+ code = SvIV(message);
+ else if (SvUOK(message))
+ code = SvUV(message);
+ else {
+ const int numtype = grok_number(SvPVX_const(message),3,&code);
+ if (numtype != IS_NUMBER_IN_UV)
+ code = 0;
+ }
+ trace2("starting response fd=%d code=%u\n",c->fd,code);
+
+ if (!code)
+ croak("first parameter is not a number or doesn't start with digits");
+
+ if (!SvPOK(message) || SvCUR(message) == 3) {
ptr = http_code_to_msg(code);
len = strlen(ptr);
message = sv_2mortal(newSVpvf("%d %.*s",code,len,ptr));
}
+
+ // don't generate or strip Content-Length headers for 304 responses.
+ c->auto_cl = (code == 304) ? 0 : 1;
add_const_to_wbuf(c, c->is_http11 ? "HTTP/1.1 " : "HTTP/1.0 ", 9);
add_sv_to_wbuf(c, message);
- add_const_to_wbuf(c, CRLF, 2);
+ add_crlf_to_wbuf(c);
for (i=0; i<avl; i+= 2) {
SV **hdr = av_fetch(headers, i, 0);
@@ -1486,15 +1505,15 @@ feersum_start_response (pTHX_ struct feer_conn *c, SV *message, AV *headers,
STRLEN hlen;
const char *hp = SvPV(*hdr, hlen);
- if (str_case_eq("content-length",14,hp,hlen)) {
+ if (c->auto_cl && str_case_eq("content-length",14,hp,hlen)) {
trace("ignoring content-length header in the response\n");
continue;
}
add_sv_to_wbuf(c, *hdr);
add_const_to_wbuf(c, ": ", 2);
add_sv_to_wbuf(c, *val);
- add_const_to_wbuf(c, CRLF, 2);
+ add_crlf_to_wbuf(c);
}
if (streaming) {
@@ -1538,7 +1557,10 @@ feersum_write_whole_body (pTHX_ struct feer_conn *c, SV *body)
SV *cl_sv; // content-length future
struct iovec *cl_iov;
- add_placeholder_to_wbuf(c, &cl_sv, &cl_iov);
+ if (c->auto_cl)
+ add_placeholder_to_wbuf(c, &cl_sv, &cl_iov);
+ else
+ add_crlf_to_wbuf(c);
if (body_is_string) {
cur = add_sv_to_wbuf(c,body);
@@ -1562,8 +1584,10 @@ feersum_write_whole_body (pTHX_ struct feer_conn *c, SV *body)
}
}
- sv_setpvf(cl_sv, "Content-Length: %d" CRLFx2, RETVAL);
- update_wbuf_placeholder(c, cl_sv, cl_iov);
+ if (c->auto_cl) {
+ sv_setpvf(cl_sv, "Content-Length: %d" CRLFx2, RETVAL);
+ update_wbuf_placeholder(c, cl_sv, cl_iov);
+ }
c->responding = RESPOND_SHUTDOWN;
conn_write_ready(c);
View
@@ -0,0 +1,69 @@
+#!perl
+use warnings;
+use strict;
+use Test::More tests => 21;
+use Test::Exception;
+use utf8;
+use lib 't'; use Utils;
+
+BEGIN { use_ok('Feersum') };
+
+my ($socket,$port) = get_listen_socket();
+ok $socket, "made listen socket";
+ok $socket->fileno, "has a fileno";
+
+my $evh = Feersum->new();
+
+$evh->request_handler(sub {
+ my $r = shift;
+ isa_ok $r, 'Feersum::Connection', 'got an object!';
+ my $env = $r->env;
+ ok $env, 'got env';
+ lives_ok {
+ if ($env->{HTTP_X_CLIENT} == 1) {
+ $r->send_response("304", [], []); # explicit string, not num
+ }
+ else {
+ $r->send_response("304 Not Modified", ['Content-Length'=>123], []);
+ }
+ } 'sent response for '.$env->{HTTP_X_CLIENT};
+});
+
+lives_ok {
+ $evh->use_socket($socket);
+} 'assigned socket';
+
+my $cv = AE::cv;
+$cv->begin;
+my $w = simple_client GET => '/?blef',
+ headers => { 'X-Client' => 1 },
+ timeout => 3,
+ sub {
+ my ($body, $headers) = @_;
+ is $headers->{Status}, 304, "client got 304";
+ ok !exists $headers->{'content-type'}, 'missing c-t';
+ # 304 not-modifieds shouldn't auto-generate a content-length header or
+ # any other "entity" headers. These reflect the actual entity, and
+ # can update cache's respresentation of the object.
+ ok !exists $headers->{'content-length'},'no c-l generated';
+ ok !$body, 'no body';
+ $cv->end;
+ };
+
+$cv->begin;
+my $w2 = simple_client GET => '/?blef',
+ headers => { 'X-Client' => 2 },
+ timeout => 3,
+ sub {
+ my ($body, $headers) = @_;
+ is $headers->{Status}, 304, "2nd client got 304";
+ ok !exists $headers->{'content-type'}, 'missing c-t';
+ # If the app specified a C-L, we should respect it for the same
+ # reasons.
+ is $headers->{'content-length'}, 123, 'c-l not replaced';
+ ok !$body, 'no body';
+ $cv->end;
+ };
+
+$cv->recv;
+pass "all done";
View
@@ -128,7 +128,14 @@ sub simple_client ($$;@) {
$hdrs{'content-length'} = 0 if ($hdrs{Status} == 204);
- if (exists $hdrs{'content-length'}) {
+ if ($hdrs{Status} == 304) {
+ # should have no body
+ $h->on_read(sub {
+ $buf .= substr($_[0]->{rbuf},0,length($_[0]->{rbuf}),'');
+ });
+ $h->on_eof($done);
+ }
+ elsif (exists $hdrs{'content-length'}) {
return $done->() unless ($hdrs{'content-length'});
# Test::More::diag "$name waiting for C-L body";
$h->push_read(chunk => $hdrs{'content-length'}, sub {

0 comments on commit 61d015b

Please sign in to comment.