From d111f763c760d076e401d2283a108ec3cd7eddd1 Mon Sep 17 00:00:00 2001 From: Andrew Bettison Date: Thu, 12 Jul 2012 12:10:59 +0930 Subject: [PATCH] Fix bugs revealed by 'rhizomeprotocol' test Was not transmitting actual HTTP server port in rhizome announcements, was always transmitting port 4110. When trying for a free HTTP server port, sometimes bind() succeeds but listen() fails with EADDRINUSE, so new logic to deal with that. --- overlay_interface.c | 2 +- rhizome.h | 1 + rhizome_http.c | 101 +++++++++++++++++++++++----------------- rhizome_packetformats.c | 3 +- serval.h | 1 + tests/rhizomeprotocol | 1 - 6 files changed, 61 insertions(+), 48 deletions(-) diff --git a/overlay_interface.c b/overlay_interface.c index 3fd04d43..0434c338 100644 --- a/overlay_interface.c +++ b/overlay_interface.c @@ -859,7 +859,7 @@ int overlay_tick_interface(int i, long long now) overlay_stuff_packet_from_queue(i,e,OQ_ORDINARY,now,pax,&frame_pax,MAX_FRAME_PAX); overlay_stuff_packet_from_queue(i,e,OQ_OPPORTUNISTIC,now,pax,&frame_pax,MAX_FRAME_PAX); /* 5. XXX Fill the packet up to a suitable size with anything that seems a good idea */ - if (rhizome_enabled()) + if (rhizome_enabled() && rhizome_http_server_running()) overlay_rhizome_add_advertisements(i,e); if (debug&DEBUG_PACKETCONSTRUCTION) diff --git a/rhizome.h b/rhizome.h index 4d18a80f..ada458be 100644 --- a/rhizome.h +++ b/rhizome.h @@ -128,6 +128,7 @@ typedef struct rhizome_manifest { extern long long rhizome_space; extern int rhizome_fetch_interval_ms; +extern unsigned short rhizome_http_server_port; int rhizome_configure(); diff --git a/rhizome_http.c b/rhizome_http.c index fc9e7e53..a0fa065b 100644 --- a/rhizome_http.c +++ b/rhizome_http.c @@ -89,9 +89,9 @@ struct profile_total connection_stats; /* HTTP server and client code for rhizome transfers. - */ +unsigned short rhizome_http_server_port = 0; static int rhizome_server_socket = -1; static long long rhizome_server_last_start_attempt = -1; @@ -120,6 +120,11 @@ unsigned char favicon_bytes[]={ ,0,0,0,0,0,0,0,0,0,0,0,0,0,0}; int favicon_len=318; +int rhizome_http_server_running() +{ + return rhizome_server_socket != -1; +} + /* Start the Rhizome HTTP server by creating a socket, binding it to an available port, and marking it as passive. If called repeatedly and frequently, this function will only try to start the server after a certain time has elapsed since the last attempt. @@ -141,54 +146,62 @@ int rhizome_http_server_start() if (debug&DEBUG_RHIZOME) DEBUGF("Starting rhizome HTTP server"); - rhizome_server_socket = socket(AF_INET,SOCK_STREAM,0); - if (rhizome_server_socket == -1) { - WHY_perror("socket"); - return WHY("Failed to start rhizome HTTP server"); - } - - int on=1; - if (setsockopt(rhizome_server_socket, SOL_SOCKET, SO_REUSEADDR, (char *)&on, sizeof(on)) == -1) { - WHY_perror("setsockopt(REUSEADDR)"); - close(rhizome_server_socket); - rhizome_server_socket = -1; - return WHY("Failed to start rhizome HTTP server"); - } - - /* Starting at the default port, look for a free port to bind to. */ - struct sockaddr_in address; - bzero((char *) &address, sizeof(address)); - address.sin_family = AF_INET; - address.sin_addr.s_addr = INADDR_ANY; - int port = RHIZOME_HTTP_PORT; - int result = -1; - do { + unsigned short port; + for (port = RHIZOME_HTTP_PORT; port <= RHIZOME_HTTP_PORT_MAX; ++port) { + /* Create a new socket, reusable and non-blocking. */ + if (rhizome_server_socket == -1) { + rhizome_server_socket = socket(AF_INET,SOCK_STREAM,0); + if (rhizome_server_socket == -1) { + WHY_perror("socket"); + goto error; + } + int on=1; + if (setsockopt(rhizome_server_socket, SOL_SOCKET, SO_REUSEADDR, (char *)&on, sizeof(on)) == -1) { + WHY_perror("setsockopt(REUSEADDR)"); + goto error; + } + if (ioctl(rhizome_server_socket, FIONBIO, (char *)&on) == -1) { + WHY_perror("ioctl(FIONBIO)"); + goto error; + } + } + /* Bind it to the next port we want to try. */ + struct sockaddr_in address; + bzero((char *) &address, sizeof(address)); + address.sin_family = AF_INET; + address.sin_addr.s_addr = INADDR_ANY; address.sin_port = htons(port); - result = bind(rhizome_server_socket, (struct sockaddr *) &address, sizeof(address)); - } while (result == -1 && errno == EADDRINUSE && ++port <= RHIZOME_HTTP_PORT_MAX); - if (result == -1) { - WHY_perror("bind"); + if (bind(rhizome_server_socket, (struct sockaddr *) &address, sizeof(address)) == -1) { + if (errno != EADDRINUSE) { + WHY_perror("bind"); + goto error; + } + } else { + /* We bound to a port. The battle is half won. Now we have to successfully listen on that + port, which could also fail with EADDRINUSE, in which case we have to scrap the socket and + create a new one, because once bound, a socket stays bound. + */ + if (listen(rhizome_server_socket, 20) != -1) + goto success; + if (errno != EADDRINUSE) { + WHY_perror("listen"); + goto error; + } + close(rhizome_server_socket); + rhizome_server_socket = -1; + } + } + WHYF("No ports available in range %u to %u", RHIZOME_HTTP_PORT, RHIZOME_HTTP_PORT_MAX); +error: + if (rhizome_server_socket != -1) { close(rhizome_server_socket); rhizome_server_socket = -1; - return WHY("Failed to start rhizome HTTP server"); - } - - if (ioctl(rhizome_server_socket, FIONBIO, (char *)&on) == -1) { - WHY_perror("ioctl(FIONBIO)"); - close(rhizome_server_socket); - rhizome_server_socket = -1; - return WHY("Failed to start rhizome HTTP server"); - } - - if (listen(rhizome_server_socket, 20) == -1) { - WHY_perror("listen"); - close(rhizome_server_socket); - rhizome_server_socket = -1; - return WHY("Failed to start rhizome HTTP server"); } + return WHY("Failed to start rhizome HTTP server"); +success: INFOF("Started Rhizome HTTP server on port %d, fd = %d", port, rhizome_server_socket); - + rhizome_http_server_port = port; /* Add Rhizome HTTPd server to list of file descriptors to watch */ server_alarm.function = rhizome_server_poll; server_stats.name="rhizome_server_poll"; @@ -196,8 +209,8 @@ int rhizome_http_server_start() server_alarm.poll.fd = rhizome_server_socket; server_alarm.poll.events = POLLIN; watch(&server_alarm); - return 0; + } void rhizome_client_poll(struct sched_ent *alarm) diff --git a/rhizome_packetformats.c b/rhizome_packetformats.c index 77c3124c..672414bd 100644 --- a/rhizome_packetformats.c +++ b/rhizome_packetformats.c @@ -76,7 +76,6 @@ int overlay_rhizome_add_advertisements(int interface_number,overlay_buffer *e) { IN(); int voice_mode=0; - unsigned short int http_port = RHIZOME_HTTP_PORT; /* behave differently during voice mode. Basically don't encourage people to grab stuff from us, but keep @@ -130,7 +129,7 @@ int overlay_rhizome_add_advertisements(int interface_number,overlay_buffer *e) */ ob_append_byte(e,3+skipmanifests); /* Rhizome HTTP server port number (2 bytes) */ - ob_append_short(e, http_port); + ob_append_short(e, rhizome_http_server_port); /* XXX Should add priority bundles here. XXX Should prioritise bundles for subscribed groups, Serval-authorised files diff --git a/serval.h b/serval.h index 3638b2f2..45f81c29 100755 --- a/serval.h +++ b/serval.h @@ -155,6 +155,7 @@ extern int returnMultiVars; extern char *gatewayspec; int rhizome_enabled(); +int rhizome_http_server_running(); const char *rhizome_datastore_path(); extern struct in_addr client_addr; diff --git a/tests/rhizomeprotocol b/tests/rhizomeprotocol index 7a847762..7c26a09b 100755 --- a/tests/rhizomeprotocol +++ b/tests/rhizomeprotocol @@ -28,7 +28,6 @@ teardown() { stop_all_servald_servers kill_all_servald_processes assert_no_servald_processes - tfw_cat $LOGA $LOGB } setup_rhizome() {