Some changes to PR #2795

* Adjusted code style
* Added unit tests
* Rewrote a function to be easier to understand
* Added support for MySQL X’val’ style hex literal

5 files changed, 242 insertions, 87 deletions

diff --git a/Source/SPDataAdditions.h b/Source/SPDataAdditions.h
index 49dcd315..a446e315 100644
--- a/Source/SPDataAdditions.h
+++ b/Source/SPDataAdditions.h
@@ -43,7 +43,7 @@ typedef NS_OPTIONS(NSUInteger, SPLineTerminator) {
 - (NSData *)dataEncryptedWithKey:(NSData *)aesKey IV:(NSData *)iv;
 - (NSData *)dataDecryptedWithPassword:(NSString *)password;
 - (NSData *)dataDecryptedWithKey:(NSData *)key;
-+ (NSData *)dataWithHexString: (NSString *)hex;
++ (NSData *)dataWithHexString:(NSString *)hex;
 
 - (NSData *)compress;
 - (NSData *)decompress;
diff --git a/Source/SPDataAdditions.m b/Source/SPDataAdditions.m
index 3a35e081..19539cd0 100644
--- a/Source/SPDataAdditions.m
+++ b/Source/SPDataAdditions.m
@@ -343,71 +343,137 @@ uint32_t LimitUInt32(NSUInteger i);
 	return hexString;
 }
 
-static int hexval( char c)
+/**
+ * Returns the integer value for a single hex-encoded nibble or -1 for invalid values.
+ * Supported characters: 0-9,a-f,A-F
+ * 
+ * Note: You usually would call this method like ((hexchar2nibble(highByte) << 4) + hexchar2nibble(lowByte)) to decode a single hex-encoded byte.
+ */
+static int hexchar2nibble(char c)
 {
-	if (c >= '0' && c <= '9')
-		return c - '0';
-	if (c >= 'a' && c <= 'f')
-		return c - 'a' + 10;
-	if (c >= 'A' && c <= 'F')
-		return c - 'A' + 10;
+	if (c >= '0' && c <= '9') return c - '0';
+	if (c >= 'a' && c <= 'f') return c - 'a' + 10;
+	if (c >= 'A' && c <= 'F') return c - 'A' + 10;
 	return -1;
 }
 
-//
-// Interpret a string of hex digits in 'hex' as hex data, and return
-// an NSData representation of the data.  Spaces are permitted within
-// the string and an initial '0x' or '0X' will be ignored. If bad input
-// is detected, nil is returned.
-//
-+ (NSData *)dataWithHexString: (NSString *)hex 
+/**
+ * Decodes a sequence of hex digits to raw byte values.
+ * This function is very strict about the allowed inputs and must only be used for validated inputs!
+ *
+ * - If numRawBytes != 0 and inBuffer == NULL or outBuffer == NULL, this will crash
+ * - The hex sequence must ONLY contain chars 0-9,a-f,A-F or the result will be undefined
+ * - The sequence must be padded to have an even length. numRawBytes is the number of bytes AFTER decoding, so inBuffer must be exactly 2x as large
+ * - inBuffer and outBuffer may be the same pointer
+ */
+static void decodeValidHexSequence(const char *inBuffer,uint8_t *outBuffer, NSUInteger numRawBytes)
 {
-	int n = (int)(hex.length + 1);
-	if (n <= 1)
-		return nil;	// no string or empty string
-	char c, *str = (char *)malloc( n), *d = str, *e;
-	const char *s = hex.UTF8String;
-	//
-	// Copy input while removing spaces and tabs.
-	//
-	do {
-		c = *s++;
-		if (c != ' ' && c != '\t')
-			*d++ = c;
-	} while (c);
-	d = str;
-	if (d[0] == '0' && (d[1] == 'x' || d[1] == 'X')) {
-		d += 2;	// bypass initial 0x or 0X
+	NSUInteger outIndex = 0;
+	NSUInteger srcIndex = 0;
+	while (outIndex < numRawBytes) {
+		uint8_t v = (hexchar2nibble(inBuffer[srcIndex]) << 4) + hexchar2nibble(inBuffer[srcIndex+1]);
+		outBuffer[outIndex++] = v;
+		srcIndex += 2;
 	}
-	//
-	// Check for non-hex characters
-	//
-	for (e = d; (c = *e); e++) {
-		if (hexval( c) < 0) {
-			break;
+}
+
+/**
+ * Interpret a string of hex digits in 'hex' as hex data, and return
+ * an NSData representation of the data.  Spaces are permitted within
+ * the string and an initial '0x' will be ignored. If bad input
+ * is detected, nil is returned.
+ *
+ * Alternatively the MySQL-style X'val' syntax is also supported, 
+ * with the same restrictions as in MySQL:
+ * - val must always be an even number of characters
+ * - val cannot contain whitespace (whitespace before/after is ok)
+ * - The leading x is case-INsensitive
+ */
++ (NSData *)dataWithHexString:(NSString *)hex
+{
+	if(!hex) return nil; // no string
+	const char *sourceBytes = [hex UTF8String];
+	
+	size_t length = strlen(sourceBytes); // keep in mind that [hex length] is the number of Unicode characters, not the number of bytes
+	if (length < 1) return [NSData data];	// empty string
+
+	NSUInteger srcIndex = 0;
+	NSData *data = nil;
+	NSUInteger nbytes;
+	
+	//skip leading whitespace (in order to properly check for leading "0x")
+	while(srcIndex < length && (sourceBytes[srcIndex] == ' ' || sourceBytes[srcIndex] == '\t')) srcIndex++;
+
+	// bypass initial 0x
+	if(srcIndex+1 < length && sourceBytes[srcIndex] == '0' && sourceBytes[srcIndex+1] == 'x' ) {
+		srcIndex += 2;
+	}
+	//check for mysql syntax
+	else if(srcIndex+2 < length && (sourceBytes[srcIndex] == 'x' || sourceBytes[srcIndex] == 'X') && sourceBytes[srcIndex+1] == '\'') {
+		srcIndex += 2;
+		//look for the terminating quote
+		NSUInteger startIndex = srcIndex;
+		NSUInteger endIndex = startIndex; //startIndex points to the first character inside the quotes, which may already be the terminating quote
+		while(endIndex < length) {
+			char c = sourceBytes[endIndex];
+			//if we've hit the terminator, verify that only whitespace follows and stop reading
+			if(c == '\'') {
+				NSUInteger afterIndex = endIndex+1;
+				while (afterIndex < length) {
+					c = sourceBytes[afterIndex++];
+					if(c != ' ' && c != '\t') return nil;
+				}
+				break;
+			}
+			endIndex++;
+			// Check for non-hex characters
+			if (hexchar2nibble(c) < 0) return nil;
 		}
+		// Check for unterminated sequence and uneven number of bytes
+		NSUInteger n = endIndex - startIndex;
+		if(endIndex == length || ((n % 2) != 0)) return nil;
+		// shortcut
+		if(n == 0) return [NSData data];
+		//looks good, create the output buffer and decode
+		nbytes = n / 2;
+		unsigned char *outBuf = malloc(nbytes);
+		decodeValidHexSequence(&sourceBytes[startIndex], outBuf, nbytes);
+		return [NSData dataWithBytesNoCopy:outBuf length:nbytes freeWhenDone:YES];
 	}
-	n = (int)(e - d);	// n = # of hex digits
-	if (*e) {
-		//
-		// Bad hex char at e. Return empty data.  Alternative would be to
-		// convert data up to bad point.
-		//
-		free( str);
-		return nil;
+	
+	// Copy input while removing spaces and tabs.
+	char *trimmedFull = (char *)malloc(length + 1);
+	char *trimmed = (trimmedFull + 1); //we'll use the first byte in case we have to fill in a leading '0'
+	NSUInteger trimIndex = 0;
+	NSUInteger n = 0; // n = # of hex digits
+	while(srcIndex < length) {
+		char c = sourceBytes[srcIndex++];
+		if(c == ' ' || c == '\t') continue;
+		trimmed[trimIndex++] = c;
+		if(!c) break;
+		n++;
+		// Check for non-hex characters
+		if (hexchar2nibble(c) < 0) goto fail_cleanup;
 	}
-	int nbytes = (n % 2) ? (n + 1) / 2 : n / 2;
-	unsigned char *bytes = malloc( nbytes), *b = bytes;
-	if (n % 2) {
-		*b++ = hexval( *d++);
+	//shortcut
+	if(n == 0) {
+		data = [NSData data];
+		goto fail_cleanup;
 	}
-	while (d < e) {
-		unsigned char v = (hexval( d[0]) << 4) + hexval( d[1]);
-		*b++ = v;
-		d += 2;
+
+	BOOL isEven = ((n % 2) == 0);
+	nbytes = !isEven ? (n + 1) / 2 : n / 2; //adjust for cases where "0aff" is written as "aff" (e.g.)
+	if(!isEven) {
+		trimmed--;
+		trimmed[0] = '0';
 	}
-	NSData *data = [NSData dataWithBytesNoCopy: bytes length: nbytes freeWhenDone: YES];
-	free( str);
+	
+	//we'll just decode the data in-place since the raw values have to be shorter by definition, anyway
+	decodeValidHexSequence(trimmed, (uint8_t *)trimmedFull, nbytes);
+	return [NSData dataWithBytesNoCopy:trimmedFull length:nbytes freeWhenDone:YES];
+	
+fail_cleanup:
+	free(trimmedFull);
 	return data;
 }
 
diff --git a/Source/SPTableContentDataSource.m b/Source/SPTableContentDataSource.m
index a005cdce..562a5856 100644
--- a/Source/SPTableContentDataSource.m
+++ b/Source/SPTableContentDataSource.m
@@ -165,10 +165,10 @@
 	}
 #endif
 	if (tableView == tableContentView) {
-		
+		NSInteger columnIndex = [[tableColumn identifier] integerValue];
 		// If the current cell should have been edited in a sheet, do nothing - field closing will have already
 		// updated the field.
-		if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:[[tableColumn identifier] integerValue] checkWithLock:NULL]) {
+		if ([tableContentView shouldUseFieldEditorForRow:rowIndex column:columnIndex checkWithLock:NULL]) {
 			return;
 		}
 		
@@ -178,11 +178,6 @@
 			return;
 		}
 		
-		NSInteger columnIndex = [[tableColumn identifier] integerValue];
-		NSDictionary *columnDefinition = [[(id <SPDatabaseContentViewDelegate>)[tableContentView delegate] dataColumnDefinitions] objectAtIndex:columnIndex];
-		
-		NSString *columnType = [columnDefinition objectForKey:@"typegrouping"];
-		
 		// Catch editing events in the row and if the row isn't currently being edited,
 		// start an edit.  This allows edits including enum changes to save correctly.
 		if (isEditingRow && [tableContentView selectedRow] != currentlyEditingRow) {
@@ -196,39 +191,29 @@
 			currentlyEditingRow = rowIndex;
 		}
 		
-		NSDictionary *column = NSArrayObjectAtIndex(dataColumns, [[tableColumn identifier] integerValue]);
+		NSDictionary *column = NSArrayObjectAtIndex(dataColumns, columnIndex);
 		
-		if ([columnType isEqualToString:@"binary"] && [object isKindOfClass: [NSString class]]) {
-			//
-			// This is a binary object being edited as a hex string. (Is there a better
-			// way to detect this case?)
-			// Convert the string back to binary, checking for errors.
-			//
-			NSData *data = [NSData dataWithHexString: object];
-			if (data) {
-				object = data;
-				[tableValues replaceObjectInRow:rowIndex column:[[tableColumn identifier] integerValue] withObject:object];
-			}
-			else {
-				SPOnewayAlertSheet(
-								   NSLocalizedString(@"Error", @"error"),
-								   [tableDocumentInstance parentWindow],
-								   NSLocalizedString(@"Bad hexadecimal data input.", @"Bad hexadecimal data input.")
-								   );
-				return;
-
-			}
-		}
-		else if (object) {
+		if (object) {
 			// Restore NULLs if necessary
 			if ([object isEqualToString:[prefs objectForKey:SPNullValue]] && [[column objectForKey:@"null"] boolValue]) {
 				object = [NSNull null];
 			}
+			else if ([self cellValueIsDisplayedAsHexForColumn:columnIndex]) {
+				// This is a binary object being edited as a hex string.
+				// Convert the string back to binary.
+				// Error checking is done in -control:textShouldEndEditing:
+				NSData *data = [NSData dataWithHexString:object];
+				if (!data) {
+					NSBeep();
+					return;
+				}
+				object = data;
+			}
 			
-			[tableValues replaceObjectInRow:rowIndex column:[[tableColumn identifier] integerValue] withObject:object];
+			[tableValues replaceObjectInRow:rowIndex column:columnIndex withObject:object];
 		} 
 		else {
-			[tableValues replaceObjectInRow:rowIndex column:[[tableColumn identifier] integerValue] withObject:@""];
+			[tableValues replaceObjectInRow:rowIndex column:columnIndex withObject:@""];
 		}
 	}
 }
diff --git a/Source/SPTableContentDelegate.m b/Source/SPTableContentDelegate.m
index 3c437333..3353a347 100644
--- a/Source/SPTableContentDelegate.m
+++ b/Source/SPTableContentDelegate.m
@@ -680,6 +680,34 @@
 #pragma mark -
 #pragma mark Control delegate methods
 
+- (BOOL)control:(NSControl *)control textShouldEndEditing:(NSText *)editor
+{
+	// Validate hex input
+	// We do this here because the textfield will still be selected with the pending changes if we bail out here
+	if(control == tableContentView) {
+		NSInteger columnIndex = [tableContentView editedColumn];
+		if ([self cellValueIsDisplayedAsHexForColumn:columnIndex]) {
+			// special case: the "NULL" string
+			NSDictionary *column = NSArrayObjectAtIndex(dataColumns, columnIndex);
+			if ([[editor string] isEqualToString:[prefs objectForKey:SPNullValue]] && [[column objectForKey:@"null"] boolValue]) {
+				return YES;
+			}
+			// This is a binary object being edited as a hex string.
+			// Convert the string back to binary, checking for errors.
+			NSData *data = [NSData dataWithHexString:[editor string]];
+			if (!data) {
+				SPOnewayAlertSheet(
+					NSLocalizedString(@"Invalid hexadecimal value", @"table content : editing : error message title when parsing as hex string failed"),
+					[tableDocumentInstance parentWindow],
+					NSLocalizedString(@"A valid hex string may only contain the numbers 0-9 and letters A-F (a-f). It can optionally begin with „0x“ and spaces will be ignored.\nAlternatively the syntax X'val' is supported, too.", @"table content : editing : error message description when parsing as hex string failed")
+				);
+				return NO;
+			}
+		}
+	}
+	return YES;
+}
+
 - (void)controlTextDidChange:(NSNotification *)notification
 {
 #ifndef SP_CODA
diff --git a/UnitTests/SPDataAdditionsTests.m b/UnitTests/SPDataAdditionsTests.m
index 4769e499..0491d4aa 100644
--- a/UnitTests/SPDataAdditionsTests.m
+++ b/UnitTests/SPDataAdditionsTests.m
@@ -41,6 +41,8 @@
 - (void)testDataDecryptedWithPassword;
 - (void)testDataDecryptedWithKey;
 - (void)testEnumerateLinesBreakingAt_withBlock;
+- (void)testDataWithHexString;
+- (void)testDataWithHexStringMySQL;
 
 @end
 
@@ -384,4 +386,78 @@
 	}
 }
 
+- (void)testDataWithHexString
+{
+	//nil
+	{
+		XCTAssertNil([NSData dataWithHexString:nil], @"nil input");
+	}
+	//empty
+	{
+		XCTAssertTrue([[NSData dataWithHexString:@""] length] == 0, @"empty input");
+	}
+	//single byte 0
+	{
+		const char single[] = {0};
+		XCTAssertEqualObjects([NSData dataWithHexString:@"0"], [NSData dataWithBytes:single length:1], @"single '0'" );
+	}
+	//empty, with 0x
+	{
+		XCTAssertEqualObjects([NSData dataWithHexString:@" 0x  "], [NSData data], @"empty input after trimming");
+	}
+	//one lower nibble
+	{
+		const char single[] = { 0xf };
+		XCTAssertEqualObjects([NSData dataWithHexString:@"0xf"], [NSData dataWithBytes:single length:1], @"0x0F");
+	}
+	//full char, uppercase
+	{
+		const char single[] = { 0xcf };
+		XCTAssertEqualObjects([NSData dataWithHexString:@"CF"], [NSData dataWithBytes:single length:1], @"0xCF");
+	}
+	//regular input
+	{
+		NSString *inp = @"0x de AD Be eF\t0102 0304";
+		const char exp[] = {0xde,0xad,0xbe,0xef,0x01,0x02,0x03,0x04};
+		XCTAssertEqualObjects([NSData dataWithHexString:inp], [NSData dataWithBytes:exp length:sizeof(exp)], @"regular input");
+	}
+	//invalid input
+	{
+		XCTAssertNil([NSData dataWithHexString:@"0xaG"], @"invalid char in input");
+	}
+}
+
+- (void)testDataWithHexStringMySQL
+{
+	//empty
+	{
+		XCTAssertEqualObjects([NSData dataWithHexString:@"x''"], [NSData data], @"empty mysql hex literal");
+	}
+	//empty, whitespace around, capital x
+	{
+		XCTAssertEqualObjects([NSData dataWithHexString:@"  X''\t  "], [NSData data], @"empty mysql hex literal (2)");
+	}
+	//nonempty valid, case-insensitive
+	{
+		const char exp[] = {0xde,0xad,0xbe,0xef};
+		XCTAssertEqualObjects([NSData dataWithHexString:@"X'deADBeeF'"], [NSData dataWithBytes:exp length:sizeof(exp)], @"regular input");
+	}
+	//bad: uneven
+	{
+		XCTAssertNil([NSData dataWithHexString:@"X'aFF'"],@"uneven length in mysql hex literal");
+	}
+	//bad: whitespace inside literal
+	{
+		XCTAssertNil([NSData dataWithHexString:@"x'0A ff'"], @"whitespace inside mysql hex literal");
+	}
+	//bad: non-whitespace after literal
+	{
+		XCTAssertNil([NSData dataWithHexString:@"X'1234'   ."], @"garbage at end");
+	}
+	//bad: non hex char in literal
+	{
+		XCTAssertNil([NSData dataWithHexString:@"x'01äß'"], @"non-hex char in literal");
+	}
+}
+
 @end