Re: [Libevent-users] Bug: evhttp_make_request(), introduced in git commit 0d6622e

[Kevin Ko <kevin.s.ko@xxxxxxxxx>]

Oh, shoot.

I had just copied and pasted the diff output from my terminal, since I wasn't sure if this mailing list would accept file attachments.  That was careless of me, since my terminal extended the line off screen.  You're right; port 80 is fine.  In fact, you might as well get rid of the three prior lines marked "auto detect a port"; that was a remnant from when I was using something other than the limited broadcast address.

I've properly attached the full patch from stable for your convenience.

Kevin

On Mon, May 23, 2011 at 1:43 AM, Nick Mathewson <nickm@xxxxxxxxxxxxx> wrote:

On Sat, May 21, 2011 at 4:06 AM, Kevin Ko <kevin.s.ko@xxxxxxxxx> wrote:
> Hi,

> Sorry for the delay.  Here's a test case.  I'm not sure if you'll like my
> use of the limited broadcast address for simulating an ENETUNREACH error
> with a TCP connection, but it's the best that I could think of.  Basically,
> we want to trigger a non-EINPROGRESS error in evutil_socket_connect()
> immediately at the connect() in order to bring about the assertion in the
> evhttp_connection_fail() error handling code.

Thanks, Kevin!  It looks good to me.  I can confirm that the test
fails with a crash on 2.0.11-stable and passes in the current
patches-2.0 branch.  Merging it.


One thing:

> +        * when working with TCP. */
> +       evcon = evhttp_connection_base_new(data->base, NULL,
> "255.255.255.255",
> +       tt_assert(evcon);

It looks like something got cut off after the ' "255.255.255.255", '
?  I added "80);" since the port shouldn't matter, but you might want
to have a look at whatever you're using to make and send patches.


yrs,
--
Nick

***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.

diff --git a/test/regress_http.c b/test/regress_http.c
index 95511ae..8d4c417 100644
--- a/test/regress_http.c
+++ b/test/regress_http.c
@@ -3008,6 +3008,60 @@ http_stream_in_cancel_test(void *arg)
 }
 
 static void
+http_connection_fail_done(struct evhttp_request *req, void *arg)
+{
+	/* An ENETUNREACH error results in an unrecoverable
+	 * evhttp_connection error (see evhttp_connection_fail()).  The
+	 * connection will be reset, and the user will be notified with a NULL
+	 * req parameter. */
+	tt_assert(!req);
+
+	test_ok = 1;
+
+ end:
+	event_base_loopexit(arg, NULL);
+}
+
+/* Test unrecoverable evhttp_connection errors by generating an ENETUNREACH
+ * error on connection. */
+static void
+http_connection_fail_test(void *arg)
+{
+	struct basic_test_data *data = arg;
+	ev_uint16_t port = 0;
+	struct evhttp_connection *evcon = NULL;
+	struct evhttp_request *req = NULL;
+
+	exit_base = data->base;
+	test_ok = 0;
+
+	/* Pick an unroutable address.  The limited broadcast address should do
+	 * when working with TCP. */
+	evcon = evhttp_connection_base_new(data->base, NULL, "255.255.255.255", 80);
+	tt_assert(evcon);
+
+	/*
+	 * At this point, we want to schedule an HTTP GET request
+	 * server using our make request method.
+	 */
+
+	req = evhttp_request_new(http_connection_fail_done, data->base);
+	tt_assert(req);
+
+	if (evhttp_make_request(evcon, req, EVHTTP_REQ_GET, "/") == -1) {
+		tt_abort_msg("Couldn't make request");
+	}
+
+	event_base_dispatch(data->base);
+
+	tt_int_op(test_ok, ==, 1);
+
+ end:
+	if (evcon)
+		evhttp_connection_free(evcon);
+}
+
+static void
 http_connection_retry_done(struct evhttp_request *req, void *arg)
 {
 	tt_assert(req);
@@ -3546,6 +3600,7 @@ struct testcase_t http_testcases[] = {
 	HTTP(stream_in),
 	HTTP(stream_in_cancel),
 
+	HTTP(connection_fail),
 	HTTP(connection_retry),
 	HTTP(data_length_constraints),