You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-cvs@httpd.apache.org by je...@apache.org on 2004/09/29 08:53:42 UTC
cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
jerenkrantz 2004/09/28 23:53:42
Modified: perl-framework/c-modules/nntp_like mod_nntp_like.c
perl-framework/t/apache contentlength.t
perl-framework/t/protocol nntp-like.t
Log:
Fix up nntp-like and content-length tests to pass.
* c-modules/nntp_like/mod_nntp_like.c: Add pre_connection hook so that the
timeout on the socket is set. (NET_TIME which normally does this is a
request filter); Minor style changes thrown in for good measure.
* t/protocol/nntp-like.t: Was t_cmp argument order swapped? The debugging
output makes no sense and leads me to believe they are reversed.
* t/apache/contentlength.t: Sending a blank C-L *should* generate a 413
(not sure what I was thinking initially why a 200 is correct); flip t_cmp
arguments and give a more descriptive error message.
Revision Changes Path
1.7 +12 -3 httpd-test/perl-framework/c-modules/nntp_like/mod_nntp_like.c
Index: mod_nntp_like.c
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/c-modules/nntp_like/mod_nntp_like.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -u -r1.6 -r1.7
--- mod_nntp_like.c 15 Jun 2004 20:20:23 -0000 1.6
+++ mod_nntp_like.c 29 Sep 2004 06:53:42 -0000 1.7
@@ -99,6 +99,12 @@
return ap_pass_brigade(c->output_filters, bb);
}
+static int nntp_like_pre_connection(conn_rec *c, void *csd)
+{
+ apr_socket_timeout_set(csd, c->base_server->keep_alive_timeout);
+ return DECLINED;
+}
+
static int nntp_like_process_connection(conn_rec *c)
{
apr_bucket_brigade *bb;
@@ -110,7 +116,7 @@
if (!cfg->enabled) {
return DECLINED;
}
-
+
/* handshake if talking over SSL */
if ((rv = nntp_like_init_connection(c)) != APR_SUCCESS) {
return rv;
@@ -123,7 +129,7 @@
do {
bb = apr_brigade_create(c->pool, c->bucket_alloc);
-
+
if ((rv = ap_get_brigade(c->input_filters, bb,
AP_MODE_GETLINE,
APR_BLOCK_READ, 0) != APR_SUCCESS ||
@@ -134,7 +140,8 @@
}
APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_flush_create(c->bucket_alloc));
- rv = ap_pass_brigade(c->output_filters, bb);
+
+ rv = ap_pass_brigade(c->output_filters, bb);
} while (rv == APR_SUCCESS);
return OK;
@@ -142,6 +149,8 @@
static void nntp_like_register_hooks(apr_pool_t *p)
{
+ ap_hook_pre_connection(nntp_like_pre_connection, NULL, NULL,
+ APR_HOOK_MIDDLE);
ap_hook_process_connection(nntp_like_process_connection,
NULL, NULL,
APR_HOOK_MIDDLE);
1.4 +4 -3 httpd-test/perl-framework/t/apache/contentlength.t
Index: contentlength.t
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/t/apache/contentlength.t,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -u -r1.3 -r1.4
--- contentlength.t 31 Jan 2004 00:25:46 -0000 1.3
+++ contentlength.t 29 Sep 2004 06:53:42 -0000 1.4
@@ -15,8 +15,8 @@
"/i_do_not_exist_in_your_wildest_imagination");
# This is expanded out.
-my @resp_strings = ("HTTP/1.1 200 OK",
- "HTTP/1.1 404 Not Found",
+my @resp_strings = ("HTTP/1.1 413 Request Entity Too Large",
+ "HTTP/1.1 413 Request Entity Too Large",
"HTTP/1.1 200 OK",
"HTTP/1.1 404 Not Found",
"HTTP/1.1 200 OK",
@@ -51,7 +51,8 @@
# Read the status line
chomp(my $response = Apache::TestRequest::getline($sock) || '');
$response =~ s/\s$//;
- ok t_cmp($resp_strings[$cycle++], $response, "response codes");
+ ok t_cmp($response, $resp_strings[$cycle++],
+ "response codes POST for $request_uri with Content-Length: $data");
do {
chomp($response = Apache::TestRequest::getline($sock) || '');
1.4 +2 -2 httpd-test/perl-framework/t/protocol/nntp-like.t
Index: nntp-like.t
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/t/protocol/nntp-like.t,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -u -r1.3 -r1.4
--- nntp-like.t 7 Aug 2004 02:15:51 -0000 1.3
+++ nntp-like.t 29 Sep 2004 06:53:42 -0000 1.4
@@ -30,7 +30,7 @@
my $response = Apache::TestRequest::getline($sock);
$response =~ s/[\r\n]+$//;
- ok t_cmp('200 localhost - ready', $response,
+ ok t_cmp($response, '200 localhost - ready',
'welcome response');
for my $data ('LIST', 'GROUP dev.httpd.apache.org', 'ARTICLE 401') {
@@ -38,6 +38,6 @@
$response = Apache::TestRequest::getline($sock);
chomp($response) if (defined($response));
- ok t_cmp($data, $response, 'echo');
+ ok t_cmp($response, $data, 'echo');
}
}
Re: cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, September 29, 2004 3:39 PM +0100 Joe Orton
<jo...@redhat.com> wrote:
> OK, the difference is in the handling of an empty Content-Length header.
> The glibc strtoll does not return an error for an empty string, as C99
> requires, and so ap_http_filter treats it exactly as "Content-Length:
> 0".
>
> I guess the strto* on your platform does return an error for this case:
> I'd say a 400 is a better error than a 413 for "Content-Length:\r\n" but
> 413 is clearly better than 200, so I've fixed ap_http_filter in HEAD.
FWIW, I had failures with Darwin and FreeBSD (might have ran it on Solaris
too, but can't recall). Yah, expecting 200 was just plainly wrong in this
case. I do think 413 is a bit arbitrary, but is clearly more correct than
200. -- justin
Re: cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Sep 29, 2004 at 10:26:05AM +0100, Joe Orton wrote:
> On Wed, Sep 29, 2004 at 06:53:42AM -0000, jerenkrantz@apache.org wrote:
> > jerenkrantz 2004/09/28 23:53:42
> >
> > Modified: perl-framework/c-modules/nntp_like mod_nntp_like.c
> > perl-framework/t/apache contentlength.t
> > perl-framework/t/protocol nntp-like.t
> > Log:
> > Fix up nntp-like and content-length tests to pass.
>
> To pass against what httpd on what platform? They were passing on Linux
> against 2.0 and HEAD. But now they're not :) Against HEAD I now get:
OK, the difference is in the handling of an empty Content-Length header.
The glibc strtoll does not return an error for an empty string, as C99
requires, and so ap_http_filter treats it exactly as "Content-Length:
0".
I guess the strto* on your platform does return an error for this case:
I'd say a 400 is a better error than a 413 for "Content-Length:\r\n" but
413 is clearly better than 200, so I've fixed ap_http_filter in HEAD.
joe
Re: cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> so yeah, it sucks, continues to suck, and I'm sorry.
ok, I went through and changed all of the ones I saw that needed changing.
all tests are fine for me except the ones for modules I don't have installed
(namely ssl and php). sorry if I broke any of those...
--Geoff
Re: cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Justin Erenkrantz wrote:
> --On Wednesday, September 29, 2004 10:26 AM +0100 Joe Orton
> <jo...@redhat.com> wrote:
>
>> Yup, the t_cmp arguments were flipped a while back.
>
>
> FWIW, I think whomever flipped the t_cmp arguments but didn't flip the
> included test cases at the same time needs a stern talking to. I spent
> over an hour and a half figuring out why the heck httpd was returning a
> 200 in that case where a 413 was clearly (or at least more) correct:
> only to find out that the debug output was swapped. Incredibly,
> incredibly lame.
yeah, well, that was me. it's difficult to find the time to do everything
that needs doing. in this case, the order was swapped to be consistent with
other (more popular) Perl testing libraries, but there just weren't enough
tuits lying around to make all the changes throughout the perl-framework.
the argument at the time was that this was OK(tm) because the only thing
affected was the debugging output, not the actual comparison. I'll take the
blame for that brain fart too, but again a severe lack of free time got in
the way of doing things a bit better.
however, those of us that are reasonably active here were aware of this, uh,
issue and were changing test files as we touched them for other reasons.
so yeah, it sucks, continues to suck, and I'm sorry. I'll buy you a beer or
two at apachecon to make up for it :)
--Geoff
Re: cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, September 29, 2004 10:26 AM +0100 Joe Orton
<jo...@redhat.com> wrote:
> Yup, the t_cmp arguments were flipped a while back.
FWIW, I think whomever flipped the t_cmp arguments but didn't flip the
included test cases at the same time needs a stern talking to. I spent
over an hour and a half figuring out why the heck httpd was returning a 200
in that case where a 413 was clearly (or at least more) correct: only to
find out that the debug output was swapped. Incredibly, incredibly lame.
All I wanted to do last night was add some caching tests: instead I had to
fix our tests to pass at all. *sigh* -- justin
Re: cvs commit: httpd-test/perl-framework/t/protocol nntp-like.t
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Sep 29, 2004 at 06:53:42AM -0000, jerenkrantz@apache.org wrote:
> jerenkrantz 2004/09/28 23:53:42
>
> Modified: perl-framework/c-modules/nntp_like mod_nntp_like.c
> perl-framework/t/apache contentlength.t
> perl-framework/t/protocol nntp-like.t
> Log:
> Fix up nntp-like and content-length tests to pass.
To pass against what httpd on what platform? They were passing on Linux
against 2.0 and HEAD. But now they're not :) Against HEAD I now get:
t/apache/contentlength......# Failed test 2 in t/apache/contentlength.t at line 54
# Failed test 4 in t/apache/contentlength.t at line 54 fail #2
FAILED tests 2, 4
Failed 2/20 tests, 90.00% okay
Yup, the t_cmp arguments were flipped a while back.