[PATCH] Improve tar method

[Christian Neumair <chris gnome-de org>]

After dedicating a notable time on an almost complete zip method, I've
spent some time on the tar method:

The attached patch significantly cleans up some weird aspects of the tar
method, for instance its inability to distinguish a 0 octal number from
an invalid or unpresent number, access of uninitialized memory and its
handling of the root node.

-- 
Christian Neumair <chris gnome-de org>

Index: modules/tar-method.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/modules/tar-method.c,v
retrieving revision 1.4
diff -u -p -r1.4 tar-method.c
--- modules/tar-method.c	29 Nov 2005 16:35:05 -0000	1.4
+++ modules/tar-method.c	17 Apr 2006 22:43:32 -0000
@@ -66,13 +66,19 @@ G_LOCK_DEFINE_STATIC (tar_cache);
 #define IS_OCTAL_DIGIT(c) ((c) >= '0' && (c) <= '8')
 #define OCTAL_DIGIT(c) ((c) - '0')
 
-static int parse_octal (const char *str, int len)
+static int inline
+parse_octal (const char *str, int len)
 {
 	int i, ret = 0;
+
+	if (str[0] == '\0') {
+		return -1;
+	}
+
 	for (i = 0; i < len; i++)
 	{
 		if (str[i] == '\0') break;
-		else if (!IS_OCTAL_DIGIT (str[i])) return 0;
+		else if (!IS_OCTAL_DIGIT (str[i])) return -1;
 		ret = ret * 8 + OCTAL_DIGIT (str[i]);
 	}
 	return ret;
@@ -155,7 +161,7 @@ real_lookup_entry (const GNode *tree, co
 }
 
 static GNode*
-tree_lookup_entry (const GNode *tree, const gchar *name)
+tree_lookup_entry (GNode *tree, const gchar *name)
 {
 	GNode *ret;
 	char *root = g_strdup (name);
@@ -164,6 +170,10 @@ tree_lookup_entry (const GNode *tree, co
 	if (txt[0] == '/')
 		txt++;
 
+	if (txt[0] == '\0') {
+		return tree;
+	}
+
 	ret = real_lookup_entry (tree, txt, 1);
 	if (!ret && txt[strlen (txt) - 1] != '/')
 	{
@@ -240,7 +250,7 @@ static TarFile* read_tar_file (GnomeVFSH
 		g_free (rest);
 	
 		maxsize = parse_octal_field (ret->blocks[i].p.size);
-		if (maxsize)
+		if (maxsize > 0)
 		{
 			for (orig = i; i < ret->num_blocks && size < maxsize; i++)
 			{
@@ -250,9 +260,10 @@ static TarFile* read_tar_file (GnomeVFSH
 				size += wsize;
 			}
 			i++;
-		}
-		else
-		{
+		} else if (maxsize < 0) {
+			g_warning ("unable to parse file size for %s", ret->blocks[i].p.name);
+			i++;
+		} else {
 			i++;
 		}
 	}
@@ -322,6 +333,7 @@ do_open (GnomeVFSMethod *method,
 	tar = ensure_tarfile (uri);
 	if (!tar)
 		return GNOME_VFS_ERROR_BAD_FILE;
+
 	node = tree_lookup_entry (tar->info_tree, uri->text);
 	if (!node)
 	{
@@ -330,7 +342,8 @@ do_open (GnomeVFSMethod *method,
 	}
 	start = node->data;
 
-	if (start->p.name[strlen (start->p.name) - 1] == '/')
+	if (start == NULL /* NULL data means root node */ ||
+	    start->p.name[strlen (start->p.name) - 1] == '/')
 		return GNOME_VFS_ERROR_IS_DIRECTORY;
 	new_handle = g_new0 (FileHandle, 1);
 	new_handle->tar = tar;
@@ -384,6 +397,10 @@ do_read (GnomeVFSMethod *method,
 		return GNOME_VFS_ERROR_IS_DIRECTORY;
 
 	maxsize = parse_octal_field (handle->start->p.size);
+	if (maxsize < 0) {
+		return GNOME_VFS_ERROR_CORRUPTED_DATA;
+	}
+
 	if (handle->current == handle->start)
 	{
 		handle->current_index++;
@@ -436,6 +453,9 @@ do_seek (GnomeVFSMethod *method,
 		break;
 	case GNOME_VFS_SEEK_END:
 		current_offset = parse_octal_field (handle->start->p.size);
+		if (current_offset < 0) {
+			return GNOME_VFS_ERROR_CORRUPTED_DATA;
+		}
 		break;
 	default:
 		current_offset = handle->current_offset;
@@ -472,40 +492,25 @@ do_open_directory (GnomeVFSMethod *metho
 	if (!uri->parent)
 		return GNOME_VFS_ERROR_INVALID_URI;
 	tar = ensure_tarfile (uri);
-	if (uri->text)
-	{
-		node = tree_lookup_entry (tar->info_tree, uri->text);
-		if (!node)
-		{
-			tar_file_unref (tar);
-			return GNOME_VFS_ERROR_NOT_FOUND;
-		}
-		start = node->data;
-		
-		if (start->p.name[strlen (start->p.name) - 1] != '/')
-			return GNOME_VFS_ERROR_NOT_A_DIRECTORY;
-
-		if (node->children)
-			current = node->children->data; 
-		else
-			current = NULL;
+	if (tar == NULL) {
+		return GNOME_VFS_ERROR_BAD_FILE;
 	}
-	else
-	{
-		node = tar->info_tree;
-		if (!node)
-		{
-			tar_file_unref (tar);
-			return GNOME_VFS_ERROR_NOT_FOUND;
-		}
 
-		if (node->children)
-			start = node->children->data;
-		else
-			start = NULL;
-		current = start;
+	node = tree_lookup_entry (tar->info_tree, uri->text);
+	if (node == NULL) {
+		tar_file_unref (tar);
+		return GNOME_VFS_ERROR_NOT_FOUND;
 	}
+	start = node->data;
 	
+	if (start != NULL && start->p.name[strlen (start->p.name) - 1] != '/')
+		return GNOME_VFS_ERROR_NOT_A_DIRECTORY;
+
+	if (node->children)
+		current = node->children->data; 
+	else
+		current = NULL;
+
 	new_handle = g_new0 (FileHandle, 1);
 	new_handle->tar = tar;
 	new_handle->filename = g_strdup (tar->filename);
@@ -546,33 +551,39 @@ do_get_file_info (GnomeVFSMethod *method
 	union TARPET_block *current;
 	gchar *name;
 	gchar *path;
-	char *mime_type;
+	const char *mime_type;
 	int i;
 
 	if (!uri->parent)
 		return GNOME_VFS_ERROR_INVALID_URI;
 
 	tar = ensure_tarfile (uri);
-	
-	if (uri->text)
-		node = tree_lookup_entry (tar->info_tree, uri->text);
-	else
-		node = tar->info_tree->children;
-	
+	if (tar == NULL) {
+		return GNOME_VFS_ERROR_BAD_FILE;
+	}
+
+	node = tree_lookup_entry (tar->info_tree, uri->text);
 	if (!node)
 	{
 		tar_file_unref (tar);
 		return GNOME_VFS_ERROR_NOT_FOUND;
 	}
 
+	name = NULL;
+
 	current = node->data;
-	for (i = 0; i < tar->num_blocks; i++)
-		if (&(tar->blocks[i]) == current)
-			break;
-	if (i && tar->blocks[i - 2].p.typeflag == TARPET_TYPE_LONGFILEN)
-		name = g_strdup (tar->blocks[i - 1].raw.data);
-	else
-		name = g_strdup (current->p.name);
+	if (current != NULL) {
+		for (i = 0; i < tar->num_blocks; i++)
+			if (&(tar->blocks[i]) == current)
+				break;
+
+		if (i >= 2 && i < tar->num_blocks && tar->blocks[i - 2].p.typeflag == TARPET_TYPE_LONGFILEN)
+			name = g_strdup (tar->blocks[i - 1].raw.data);
+		else
+			name = g_strdup (current->p.name);
+	} else { /* root node */
+		name = g_strdup ("/");
+	}
 
 	file_info->name = g_path_get_basename (name);
 	if (name[strlen (name) - 1] == '/')
@@ -585,13 +596,38 @@ do_get_file_info (GnomeVFSMethod *method
 	else
 		file_info->type = GNOME_VFS_FILE_TYPE_REGULAR;
 
-	file_info->permissions = parse_octal_field (current->p.mode);
-	file_info->uid = parse_octal_field (current->p.uid);
-	file_info->gid = parse_octal_field (current->p.gid);
-	file_info->size = parse_octal_field (current->p.size);
-	file_info->mtime = parse_octal_field (current->p.mtime);
-	file_info->atime = parse_octal_field (current->gnu.atime);
-	file_info->ctime = parse_octal_field (current->gnu.ctime);
+	if (current != NULL) {
+		file_info->permissions = parse_octal_field (current->p.mode);
+		if (!(file_info->permissions < 0)) {
+			file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_PERMISSIONS;
+		}
+
+		file_info->uid = parse_octal_field (current->p.uid);
+		file_info->gid = parse_octal_field (current->p.gid);
+		if (!(file_info->uid < 0) && !(file_info->gid < 0)) {
+			file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_IDS;
+		}
+
+		file_info->size = parse_octal_field (current->p.size);
+		if (!(file_info->size < 0)) {
+			file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_SIZE;
+		}
+
+		file_info->mtime = parse_octal_field (current->p.mtime);
+		if (!(file_info->mtime < 0)) {
+			file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_MTIME;
+		}
+
+		file_info->atime = parse_octal_field (current->gnu.atime);
+		if (!(file_info->atime < 0)) {
+			file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_ATIME;
+		}
+
+		file_info->ctime = parse_octal_field (current->gnu.ctime);
+		if (!(file_info->ctime < 0)) {
+			file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_CTIME;
+		}
+	}
 
 	if (file_info->type == GNOME_VFS_FILE_TYPE_DIRECTORY)
 		mime_type = "x-directory/normal";
@@ -616,14 +652,8 @@ do_get_file_info (GnomeVFSMethod *method
 
 	file_info->mime_type = g_strdup (mime_type);
 
-	file_info->valid_fields = GNOME_VFS_FILE_INFO_FIELDS_TYPE |
-			   	  GNOME_VFS_FILE_INFO_FIELDS_PERMISSIONS |
-			   	  GNOME_VFS_FILE_INFO_FIELDS_SIZE |
-			   	  GNOME_VFS_FILE_INFO_FIELDS_ATIME |
-			   	  GNOME_VFS_FILE_INFO_FIELDS_MTIME |
-			   	  GNOME_VFS_FILE_INFO_FIELDS_CTIME |
-			   	  GNOME_VFS_FILE_INFO_FIELDS_MIME_TYPE |
-				  GNOME_VFS_FILE_INFO_FIELDS_IDS;
+	file_info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_TYPE |
+				   GNOME_VFS_FILE_INFO_FIELDS_MIME_TYPE;
 
 	g_free (name);
 	tar_file_unref (tar);