From 03cebdcc03f7d8d297b56ebfe640e5496f415b0b Mon Sep 17 00:00:00 2001 From: Colin Cogle Date: Tue, 29 Oct 2024 20:32:21 -0400 Subject: [PATCH] Make I/O non-blocking, add --timeout parameter. Thank you to DL9SEC for reporting a strange case. The IGate server cwop.aprs2.net has multiple IP addresses defined in DNS, and not all of them work. This causes aprs-weather-submit to hang. This commit converts all socket operations to non-blocking. I've also added a new --timeout parameter that can specify how many seconds the app will wait before trying the next IPv4/IPv6 address, or failing. The default is 15 seconds, because that sounded nice. This does add a few kilobytes to the compiled app, so I may wrap this in a `#ifdef ONLY_BLOCKING_IO` or something. I'm still wondering if this is major-enough change to warrant a bump of the version number to 2.0.0. Fixes: #12 --- CHANGELOG.md | 2 ++ NEWS.md | 16 +++++------- configure.ac | 5 ++++ man/aprs-weather-submit.man | 16 ++++++++++-- src/aprs-is.c | 49 +++++++++++++++++++++++++++++++++---- src/aprs-is.h | 11 ++++++++- src/help.c | 1 + src/main.c | 13 ++++++++-- 8 files changed, 93 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0cf7e7..2ef9d56 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Change Log for `aprs-weather-submit` ## Latest changes +* Converted the APRS-IS socket from blocking to non-blocking, with a default timeout of 15 seconds before the connection is aborted. +* Added a new parameter, `--timeout`, to set how long the app will wait for the remote server before giving up. * Added support for ISO-compliant compilers (e.g., `gcc -pedantic`). * Fixed a bug where `./configure --disable-aprs-is` was not honored. diff --git a/NEWS.md b/NEWS.md index 80d22c9..9cf093c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,20 +2,16 @@ This file details all of the user-facing changes that are in versions 1.7 of aprs-weather-submit. For more details, please consult the CHANGELOG file or this project's GitHub page. -## Custom Icons +## Graceful failure -The new `--icon` parameter lets you specify a character from the two symbol tables. If not specified, you will get the classic weather icon. +In version 1.7.2 and older, the app would hang almost indefinitely if the remote APRS-IS server did not respond. This version fixes that by converting to non-blocking sockets, so that if your APRS-IS server does not respond in a timely manner (for whatever reason), the connection will fail gracefully. -Note that APRS.fi does not support parsing weather data from packets that have custom icons. If this is important to you, do not use this new feature. +Likewise, there is a new parameter, `--timeout`, that will let you specify a timeout value, if you don't like my default of fifteen seconds. -## `armhf` Fixes +If you want the old behavior, specify `--timeout 0`. -Version 1.7 did not compile on the armhf platforms. Version 1.7.1 is fixed. -## It's not the heat, it's the humidity! +## Building without APRS-IS support -Before version 1.7.2, one could not create an APRS packet with 100% relative humidity due to a bug in the code that only allowed values from 1% to 99%. This was corrected in version 1.7.2 thanks to a report from Jeff, N9CQS. +I'd always given users the option to build a custom version without APRS-IS support by specifying `./configure --disable-aprs-is`. However, that broke at some point. This option now works as intended. -## Can this app be in Debian, please? - -I've also been making behind-the-scenes improvements to how this app is packaged and installed. Users of Debian-based Linux distributions can enjoy nearly-effortless installation, upgrades, and removals. diff --git a/configure.ac b/configure.ac index 7fd9f23..99c2b8e 100644 --- a/configure.ac +++ b/configure.ac @@ -132,6 +132,11 @@ dnl need: [gai_strerror getaddrinfo inet_ntop ntohs perror recv send shutdown so [], AC_MSG_ERROR([Cannot compile with APRS-IS support because is missing.]) ) + AC_CHECK_FUNC( + [atol], + [], + AC_MSG_ERROR([atol() is required to build this package but was not found in stdlib.h.]) + ) AS_IF([test "x$enable_windows" != "xyes"], [ AC_CHECK_HEADERS( [arpa/inet.h], diff --git a/man/aprs-weather-submit.man b/man/aprs-weather-submit.man index 9fdfe97..2ce88e3 100644 --- a/man/aprs-weather-submit.man +++ b/man/aprs-weather-submit.man @@ -18,7 +18,7 @@ .\" .\" (This page is best viewed with the command: groff -man) .\" -.TH aprs\-weather\-submit 1 "2023-11-30" "Version 1.7.1" "aprs-weather-submit General Help" +.TH aprs\-weather\-submit 1 "2024-10-29" "Version 1.7.2-git" "aprs-weather-submit General Help" .SH NAME aprs\-weather\-submit \- manually submit weather station data to the APRS-IS network .SH DESCRIPTION @@ -34,6 +34,8 @@ This command-line app will manually submit APRS 1.2.1-compliant weather informat [ .BI "\-I " SERVER_NAME " \-o " PORT_NUMBER " \-u " USERNAME " \-d " PASSWORD ] +.RB [ " \-m " +.IR SECONDS " ]\:" .RB [ " \-A " .IR FEET " ]\:" .RB [ " \-b " @@ -145,6 +147,16 @@ Authenticate to the server with this password. Note that this will be sent in cleartext! (See the BUGS AND ERRATA section.) +.SS OPTIONAL APRS-IS OPTIONS +By default, the app will wait 15 seconds for the IGate server to respond before giving up. +.TP +.BR \-m ", " \-\-timeout =\fISECONDS\fP +Wait this many seconds before aborting the connection. +The default is 15 seconds, but you may want to use a longer value over a slow Internet connection. +.TP +To wait forever and block until the server responds (or your operating system terminates the connection), specify a value of 0 seconds. +This was the behavior in previous versions of the app. + .SS WEATHER OPTIONS You may add zero or more weather measurements to your report. All minimum and maximum measurements listed below are inclusive unless otherwise specified. @@ -298,7 +310,7 @@ APRS Version 1.2.1, "Weather Updates to the Spec" (24 Mar 2011) .UE .SH AUTHOR AND COPYRIGHT -.BR aprs\-weather\-submit ", version 1.6" +.BR aprs\-weather\-submit ", version 1.7.2-git" .br Copyright (c) 2019-2024 Colin Cogle. .br diff --git a/src/aprs-is.c b/src/aprs-is.c index df88cd5..d170e78 100755 --- a/src/aprs-is.c +++ b/src/aprs-is.c @@ -26,11 +26,13 @@ with this program. If not, see . #include /* strcat() */ #include "main.h" /* PACKAGE, VERSION */ #include "aprs-is.h" +#include /* errno */ #ifndef _WIN32 #include /* size_t */ #include /* socket(), connect(), shutdown(), recv(), send() */ #include /* sockaddr, sockaddr_in, sockaddr_in6 */ +#include /* TCP_USER_TIMEOUT */ #include /* inet_pton() */ #include /* getaddrinfo(), gai_strerror() */ #include /* EAI_SYSTEM */ @@ -45,6 +47,7 @@ with this program. If not, see . * @author Colin Cogle * @param server The DNS hostname of the server. * @param port The listening port on the server. + * @param timeout How long to wait before ending the connection. * @param username The username with which to authenticate to the server. * @param password The password with which to authenticate to the server. * @param toSend The APRS-IS packet, as a string. @@ -53,6 +56,7 @@ with this program. If not, see . void sendPacket (const char* const restrict server, const unsigned short port, + const time_t timeout, const char* const restrict username, const char* const restrict password, const char* const restrict toSend) @@ -124,6 +128,14 @@ sendPacket (const char* const restrict server, } /* Connect */ + char* timeoutText = malloc(BUFSIZE); + sprintf(timeoutText, " (timeout %ld seconds)", timeout); + + if (timeout <= 0) + { + strncpy(timeoutText, " (no timeout)", BUFSIZE); + } + switch (addressinfo->sa_family) { case AF_INET: @@ -133,10 +145,14 @@ sendPacket (const char* const restrict server, buffer, INET_ADDRSTRLEN); #ifdef DEBUG - printf("Connecting to %s:%d...\n", buffer, - ntohs(((struct sockaddr_in*)addressinfo)->sin_port)); + printf("Connecting to %s:%d%s...\n", + buffer, + ntohs(((struct sockaddr_in*)addressinfo)->sin_port), + timeoutText + ); #endif break; + case AF_INET6: inet_ntop( AF_INET6, @@ -144,11 +160,26 @@ sendPacket (const char* const restrict server, buffer, INET6_ADDRSTRLEN); #ifdef DEBUG - printf("Connecting to [%s]:%d...\n", buffer, - ntohs(((struct sockaddr_in6*)addressinfo)->sin6_port)); + printf("Connecting to [%s]:%d%s...\n", + buffer, + ntohs(((struct sockaddr_in6*)addressinfo)->sin6_port), + timeoutText + ); #endif break; } + free(timeoutText); + + /* Code to handle timeouts, as long as the user doesn't + want to wait forever (like v1.7.2 and older). */ + if (timeout > 0) + { + struct timeval socket_timeout; + socket_timeout.tv_sec = timeout; + socket_timeout.tv_usec = 0; + setsockopt(socket_desc, SOL_SOCKET, SO_SNDTIMEO, &socket_timeout, sizeof(socket_timeout)); + setsockopt(socket_desc, SOL_SOCKET, SO_RCVTIMEO, &socket_timeout, sizeof(socket_timeout)); + } if (connect(socket_desc, addressinfo, (size_t)(result->ai_addrlen)) >= 0) { @@ -157,6 +188,14 @@ sendPacket (const char* const restrict server, } else { + /* The error message for a timeout is misleading. + It says the connection is in progress, but we + are going to kill it, so this next statement + replaces that error with something clearer. */ + if (errno == EINPROGRESS) + { + errno = ETIMEDOUT; + } perror("error in connect()"); shutdown(socket_desc, 2); } @@ -207,7 +246,7 @@ sendPacket (const char* const restrict server, printf("> %s", toSend); #endif send(socket_desc, toSend, (size_t)strlen(toSend), 0); - + /* Done! */ shutdown(socket_desc, 2); return; diff --git a/src/aprs-is.h b/src/aprs-is.h index 8468074..b455c51 100755 --- a/src/aprs-is.h +++ b/src/aprs-is.h @@ -30,17 +30,26 @@ with this program. If not, see . * @author Colin Cogle * @param server The DNS hostname of the server. * @param port The listening port on the server. + * @param timeout How long to wait before timing out. * @param username The username with which to authenticate to the server. * @param password The password with which to authenticate to the server. * @param toSend The APRS-IS packet, as a string. * @since 0.3 */ void -sendPacket (const char* const restrict server, const unsigned short port, +sendPacket (const char* const restrict server, + const unsigned short port, + const time_t timeout, const char* const restrict username, const char* const restrict password, const char* const restrict toSend); +/* How long to wait before timing out when the user doesn't specify. + Fifteen seconds seems reasonable. */ +#ifndef DEFAULT_TIMEOUT +#define DEFAULT_TIMEOUT 15 +#endif + /* This should be defined by the operating system, but just in case... */ #ifndef NI_MAXHOST #define NI_MAXHOST 1025 diff --git a/src/help.c b/src/help.c index 0d06358..b14616e 100644 --- a/src/help.c +++ b/src/help.c @@ -98,6 +98,7 @@ Required parameters:\n\ puts("APRS-IS IGate parameters:\n\ -I, --server Name of the APRS-IS IGate server to submit the packet to.\n\ -o, --port Port that the APRS-IS IGate service is listening on.\n\ + -m, --timeout How many seconds to wait for a connection.\n\ -u, --username Authenticate to the server with this username.\n\ -d, --password Authenticate to the server with this password.\n"); #endif /* HAVE_APRSIS_SUPPORT */ diff --git a/src/main.c b/src/main.c index 49e6e4d..93dc56d 100755 --- a/src/main.c +++ b/src/main.c @@ -61,6 +61,7 @@ main (const int argc, const char** argv) char username[BUFSIZE] = ""; char password[BUFSIZE] = ""; char server[NI_MAXHOST] = ""; + time_t timeout = DEFAULT_TIMEOUT; uint16_t port = 0; #endif @@ -75,6 +76,7 @@ main (const int argc, const char** argv) #ifdef HAVE_APRSIS_SUPPORT {"server", required_argument, 0, 'I'}, {"port", required_argument, 0, 'o'}, + {"timeout", required_argument, 0, 'm'}, {"username", required_argument, 0, 'u'}, {"password", required_argument, 0, 'd'}, #endif @@ -131,8 +133,10 @@ main (const int argc, const char** argv) #ifdef _DOS while ((c = (char) getopt(argc, (char**)argv, "CH?vI:o:u:d:k:n:e:A:c:S:g:t:T:r:P:p:s:h:b:L:X:F:V:QM:i:")) != -1) +#elif HAVE_APRSIS_SUPPORT + while ((c = (char) getopt_long(argc, (char* const*)argv, "CHvI:o:m:u:d:k:n:e:A:c:S:g:t:T:r:P:p:s:h:b:L:X:F:V:QM:i:", long_options, &option_index)) != -1) #else - while ((c = (char) getopt_long(argc, (char* const*)argv, "CHvI:o:u:d:k:n:e:A:c:S:g:t:T:r:P:p:s:h:b:L:X:F:V:QM:i:", long_options, &option_index)) != -1) + while ((c = (char) getopt_long(argc, (char* const*)argv, "CHvk:n:e:A:c:S:g:t:T:r:P:p:s:h:b:L:X:F:V:QM:i:", long_options, &option_index)) != -1) #endif { double x = 0.0; /* scratch space */ @@ -182,6 +186,11 @@ main (const int argc, const char** argv) port = (unsigned short)x; break; + /* IGate server timeout (-m | --timeout) */ + case 'm': + timeout = (time_t)atol(optarg); + break; + /* IGate server username (-u | --username) */ case 'u': snprintf_verify( @@ -646,7 +655,7 @@ main (const int argc, const char** argv) */ if (strlen(server) && strlen(username) && strlen(password) && port != 0) { - sendPacket(server, port, username, password, packetToSend); + sendPacket(server, port, timeout, username, password, packetToSend); } else {