Issue 84. IGate was truncating packets that contained nul character

in the information/comment part.

Author: wb2osz

CHANGES.md

@@ -2,6 +2,23 @@
 # Revision History #
 
 
+## Version 1.4  -- Development snapshot H --  March 2017 ##
+
+**This is beta test quality.  If no significant issues are reported this will be the version 1.4 release.**
+
+### New Features: ###
+
+- Take advantage of new 'gpio' group and new /sys/class/gpio ownership in Raspbian Jessie.
+
+- Handle more complicated gpio naming for CubieBoard, etc.
+
+ 
+### Bugs Fixed: ###
+
+- IGate did not retain nul characters in the information part of a packet.  This should never happen with a valid APRS packet but there are a couple cases where it has.  If we encounter these malformed packets, pass them along as-is, rather than truncating.
+
+- Don't digipeat packets when the source is my call.
+
 ----------
 
 ## Version 1.4  -- Development snapshot G --  January 2017 ##

ax25_pad.c

@@ -335,9 +335,18 @@ void ax25_delete (packet_t this_p)
  * Purpose:	Parse a frame in human-readable monitoring format and change
  *		to internal representation.
  *
- * Input:	monitor	- "TNC-2" format of a monitored packet.  i.e.
+ * Input:	monitor	- "TNC-2" monitor format for packet.  i.e.
  *				source>dest[,repeater1,repeater2,...]:information
  *
+ *			The information part can have non-printable characters
+ *			in the form of <0xff>.  This will be converted to single
+ *			bytes.  e.g.  <0x0d> is carriage return.
+ *			In version 1.4H we will allow nul characters which means
+ *			we have to maintain a length rather than using strlen().
+ *			I maintain that it violates the spec but want to handle it
+ *			because it does happen and we want to preserve it when
+ *			acting as an IGate rather than corrupting it.
+ *
  *		strict	- True to enforce rules for packets sent over the air.
  *			  False to be more lenient for packets from IGate server.
  *
@@ -369,17 +378,11 @@ packet_t ax25_from_text (char *monitor, int strict)
 	char *pa;
 	char *saveptr;		/* Used with strtok_r because strtok is not thread safe. */
 
-	static int first_time = 1;
-	static regex_t unhex_re;
-	int e;
-	char emsg[100];
-#define MAXMATCH 1
-	regmatch_t match[MAXMATCH];
-	int keep_going;
-	char temp[512];
 	int ssid_temp, heard_temp;
 	char atemp[AX25_MAX_ADDR_LEN];
 
+	char info_part[AX25_MAX_INFO_LEN+1];
+	int info_len;
 
 	packet_t this_p = ax25_new ();
 
@@ -392,58 +395,15 @@ packet_t ax25_from_text (char *monitor, int strict)
 
 	/* Is it possible to have a nul character (zero byte) in the */
 	/* information field of an AX.25 frame? */
-	/* Yes, but it would be difficult in the from-text case. */
+	/* At this point, we have a normal C string. */
+	/* It is possible that will convert <0x00> to a nul character later. */
+	/* There we need to maintain a separate length and not use normal C string functions. */
 
 	strlcpy (stuff, monitor, sizeof(stuff));
 
-/* 
- * Translate hexadecimal values like <0xff> to non-printing characters.
- * MIC-E message type uses 5 different non-printing characters.
- */
-
-	if (first_time) 
-	{
-	  e = regcomp (&unhex_re, "<0x[0-9a-fA-F][0-9a-fA-F]>", 0);
-	  if (e) {
-	    regerror (e, &unhex_re, emsg, sizeof(emsg));
-	    text_color_set(DW_COLOR_ERROR);
-	    dw_printf ("%s:%d: %s\n", __FILE__, __LINE__, emsg);
-	  }
-
-	  first_time = 0;
-	}
-
-#if 0
-	text_color_set(DW_COLOR_DEBUG);
-	dw_printf ("BEFORE: %s\n", stuff);
-	ax25_safe_print (stuff, -1, 0);
-	dw_printf ("\n");
-#endif
-	keep_going = 1;
-	while (keep_going) {
-	  if (regexec (&unhex_re, stuff, MAXMATCH, match, 0) == 0) {
-	    int n;
-	    char *p;
-  
-	    stuff[match[0].rm_so + 5] = '\0';
-	    n = strtol (stuff + match[0].rm_so + 3, &p, 16);
-	    stuff[match[0].rm_so] = n;
-	    strlcpy (temp, stuff + match[0].rm_eo, sizeof(temp));
-	    strlcpy (stuff + match[0].rm_so + 1, temp, sizeof(stuff)-match[0].rm_so-1);
-	  }
-	  else {
-	    keep_going = 0;
-	  }
-	}
-#if 0
-	text_color_set(DW_COLOR_DEBUG);
-	dw_printf ("AFTER:  %s\n", stuff);
-	ax25_safe_print (stuff, -1, 0);
-	dw_printf ("\n");
-#endif
 
 /*
- * Initialize the packet with two addresses and control/pid
+ * Initialize the packet structure with two addresses and control/pid
  * for APRS.
  */
 	memset (this_p->frame_data + AX25_DESTINATION*7, ' ' << 1, 6);
@@ -473,12 +433,6 @@ packet_t ax25_from_text (char *monitor, int strict)
 	*pinfo = '\0';
 	pinfo++;
 
-	if (strlen(pinfo) > AX25_MAX_INFO_LEN) {
-	  text_color_set(DW_COLOR_ERROR);
-	  dw_printf ("Warning: Information part truncated to %d characters.\n", AX25_MAX_INFO_LEN);
-	  pinfo[AX25_MAX_INFO_LEN] = '\0';
-	}
-	
 /*
  * Separate the addresses.
  * Note that source and destination order is swappped.
@@ -535,7 +489,6 @@ packet_t ax25_from_text (char *monitor, int strict)
  */
 	while (( pa = strtok_r (NULL, ",", &saveptr)) != NULL && this_p->num_addr < AX25_MAX_ADDRS ) {
 
-	  //char *last;
 	  int k;
 
 	  k = this_p->num_addr;
@@ -561,11 +514,61 @@ packet_t ax25_from_text (char *monitor, int strict)
 	  }
         }
 
+
+/*
+ * Finally, process the information part.
+ *
+ * Translate hexadecimal values like <0xff> to single bytes.
+ * MIC-E format uses 5 different non-printing characters.
+ * We might want to manually generate UTF-8 characters such as degree.
+ */
+
+//#define DEBUG14H 1
+
+#if DEBUG14H
+	text_color_set(DW_COLOR_DEBUG);
+	dw_printf ("BEFORE: %s\nSAFE:   ", pinfo);
+	ax25_safe_print (pinfo, -1, 0);
+	dw_printf ("\n");
+#endif
+
+	info_len = 0;
+	while (*pinfo != '\0' && info_len < AX25_MAX_INFO_LEN) {
+
+	  if (strlen(pinfo) >= 6 &&
+		pinfo[0] == '<' &&
+		pinfo[1] == '0' &&
+		pinfo[2] == 'x' &&
+		isxdigit(pinfo[3]) &&
+		isxdigit(pinfo[4]) &&
+		pinfo[5] == '>') {
+
+	    char *p;
+
+	    info_part[info_len] = strtol (pinfo + 3, &p, 16);
+	    info_len++;
+	    pinfo += 6;
+	  }
+	  else {
+	    info_part[info_len] = *pinfo;
+	    info_len++;
+	    pinfo++;
+	  }
+	}
+	info_part[info_len] = '\0';
+
+#if DEBUG14H
+	text_color_set(DW_COLOR_DEBUG);
+	dw_printf ("AFTER:  %s\nSAFE:   ", info_part);
+	ax25_safe_print (info_part, info_len, 0);
+	dw_printf ("\n");
+#endif
+
 /*
  * Append the info part.  
  */
-	strlcpy ((char*)(this_p->frame_data+this_p->frame_len), pinfo, sizeof(this_p->frame_data)-this_p->frame_len);
-	this_p->frame_len += strlen(pinfo);
+	memcpy ((char*)(this_p->frame_data+this_p->frame_len), info_part, info_len);
+	this_p->frame_len += info_len;
 
 	return (this_p);
 }
@@ -2536,7 +2539,7 @@ unsigned short ax25_m_m_crc (packet_t pp)
  *
  * Inputs:	pstr	- Pointer to string.
  *
- *		len	- Maximum length if not -1.
+ *		len	- Number of bytes.  If < 0 we use strlen().
  *
  *		ascii_only	- Restrict output to only ASCII.
  *				  Normally we allow UTF-8.
@@ -2587,7 +2590,6 @@ void ax25_safe_print (char *pstr, int len, int ascii_only)
 	if (len > MAXSAFE)
 	  len = MAXSAFE;
 
-	//while (len > 0 && *pstr != '\0')
 	while (len > 0)
 	{
 	  ch = *((unsigned char *)pstr);

decode_aprs.c

@@ -199,6 +199,33 @@ void decode_aprs (decode_aprs_t *A, packet_t pp, int quiet)
 	ax25_get_addr_with_ssid (pp, AX25_SOURCE, A->g_src);
 	ax25_get_addr_with_ssid (pp, AX25_DESTINATION, dest);
 
+/*
+ * Report error if the information part contains a nul character.
+ * There are two known cases where this can happen.
+ *
+ *  - The Kenwood TM-D710A sometimes sends packets like this:
+ *
+ * 	VA3AJ-9>T2QU6X,VE3WRC,WIDE1,K8UNS,WIDE2*:4P<0x00><0x0f>4T<0x00><0x0f>4X<0x00><0x0f>4\<0x00>`nW<0x1f>oS8>/]"6M}driving fast= 
+ * 	K4JH-9>S5UQ6X,WR4AGC-3*,WIDE1*:4P<0x00><0x0f>4T<0x00><0x0f>4X<0x00><0x0f>4\<0x00>`jP}l"&>/]"47}QRV from the EV =
+ *
+ *     Notice that the data type indicator of "4" is not valid.  If we remove
+ *     4P<0x00><0x0f>4T<0x00><0x0f>4X<0x00><0x0f>4\<0x00>   we are left with a good MIC-E format.
+ *     This same thing has been observed from others and is intermittent.
+ *
+ *  - AGW Tracker can send UTF-16 if an option is selected.  This can introduce nul bytes.
+ *    This is wrong, it should be using UTF-8.
+ */
+
+	if ( ( ! A->g_quiet ) && ( (int)strlen((char*)pinfo) != info_len) ) {
+
+	  text_color_set(DW_COLOR_ERROR);
+	  dw_printf("'nul' character found in Information part.  This should never happen.\n");
+	  dw_printf("It seems that %s is transmitting with defective software.\n", A->g_src);
+
+	  if (strcmp((char*)pinfo, "4P") == 0) {
+	    dw_printf("The TM-D710 will do this intermittently.  A firmware upgrade is needed to fix it.\n");
+	  }
+	}
 
 	switch (*pinfo) {	/* "DTI" data type identifier. */
 
@@ -3907,7 +3934,7 @@ static void substr_se (char *dest, const char *src, int start, int endp1)
  *		clen		- Length of comment or -1 to take it all.
  *
  * Outputs:	A->g_telemetry	- Base 91 telemetry |ss1122|
- *		A->g_altitude_ft	- from /A=123456
+ *		A->g_altitude_ft - from /A=123456
  *		A->g_lat	- Might be adjusted from !DAO!
  *		A->g_lon	- Might be adjusted from !DAO!
  *		A->g_aprstt_loc	- Private extension to !DAO!
@@ -3938,6 +3965,49 @@ static void substr_se (char *dest, const char *src, int start, int endp1)
  *
  *			/A=123456		Altitude
  *
+ * What can appear in a comment?
+ *
+ *		Chapter 5 of the APRS spec ( http://www.aprs.org/doc/APRS101.PDF ) says:
+ *
+ *			"The comment may contain any printable ASCII characters (except | and ~,
+ *			which are reserved for TNC channel switching)."
+ *
+ *		"Printable" would exclude character values less than space (00100000), e.g.
+ *		tab, carriage return, line feed, nul.  Sometimes we see carriage return
+ *		(00001010) at the end of APRS packets.   This would be in violation of the
+ *		specification.
+ *
+ *		The base 91 telemetry format (http://he.fi/doc/aprs-base91-comment-telemetry.txt ),
+ *		which is not part of the APRS spec, uses the | character in the comment to delimit encoded
+ *		telemetry data.   This would be in violation of the original spec.
+ *
+ *		The APRS Spec Addendum 1.2 Proposals ( http://www.aprs.org/aprs12/datum.txt)
+ *		adds use of UTF-8 (https://en.wikipedia.org/wiki/UTF-8 )for the free form text in
+ *		messages and comments. It can't be used in the fixed width fields.
+ *
+ *		Non-ASCII characters are represented by multi-byte sequences.  All bytes in these
+ *		multi-byte sequences have the most significant bit set to 1.  Using UTF-8 would not
+ *		add any nul (00000000) bytes to the stream.
+ *
+ *		There are two known cases where we can have a nul character value.
+ *
+ *		* The Kenwood TM-D710A sometimes sends packets like this:
+ *
+ *			VA3AJ-9>T2QU6X,VE3WRC,WIDE1,K8UNS,WIDE2*:4P<0x00><0x0f>4T<0x00><0x0f>4X<0x00><0x0f>4\<0x00>`nW<0x1f>oS8>/]"6M}driving fast= 
+ *			K4JH-9>S5UQ6X,WR4AGC-3*,WIDE1*:4P<0x00><0x0f>4T<0x00><0x0f>4X<0x00><0x0f>4\<0x00>`jP}l"&>/]"47}QRV from the EV =
+ *
+ *		  Notice that the data type indicator of "4" is not valid.  If we remove
+ *		  4P<0x00><0x0f>4T<0x00><0x0f>4X<0x00><0x0f>4\<0x00>   we are left with a good MIC-E format.
+ *		  This same thing has been observed from others and is intermittent.
+ *
+ *		* AGW Tracker can send UTF-16 if an option is selected.  This can introduce nul bytes.
+ *		  This is wrong.  It should be using UTF-8 and I'm not going to accomodate it here.
+ *
+ *
+ *		The digipeater and IGate functions should pass along anything exactly the
+ *		we received it, even if it is invalid.  If different implementations try to fix it up
+ *		somehow, like changing unprintable characters to spaces, we will only make things
+ *		worse and thwart the duplicate detection.
  *
  *------------------------------------------------------------------*/
 

direwolf.c

@@ -260,7 +260,7 @@ int main (int argc, char *argv[])
 	text_color_init(t_opt);
 	text_color_set(DW_COLOR_INFO);
 	//dw_printf ("Dire Wolf version %d.%d (%s) Beta Test\n", MAJOR_VERSION, MINOR_VERSION, __DATE__);
-	dw_printf ("Dire Wolf DEVELOPMENT version %d.%d %s (%s)\n", MAJOR_VERSION, MINOR_VERSION, "G", __DATE__);
+	dw_printf ("Dire Wolf DEVELOPMENT version %d.%d %s (%s)\n", MAJOR_VERSION, MINOR_VERSION, "H", __DATE__);
 	//dw_printf ("Dire Wolf version %d.%d\n", MAJOR_VERSION, MINOR_VERSION);
 
 #if defined(ENABLE_GPSD) || defined(USE_HAMLIB)