[pacman-dev] [PATCH 3/3] Don't trim whitespace when reading database entries

30 Jul 2011

We don't write with extra or unknown whitespace, so there is little
reason for us to trim it when reading either. This also fixes the
hopefully never encountered "paths that start or end with spaces" issue,
for which two pactests have been added. The tests also contain other
evil characters that we have encountered before and handle just fine,
but it doesn't hurt to ensure we don't break such support in the future.

Signed-off-by: Dan McGee <dan@archlinux.org>
---
 lib/libalpm/be_local.c         |   12 ++++----
 lib/libalpm/be_package.c       |    7 ++---
 lib/libalpm/be_sync.c          |   12 +++++----
 lib/libalpm/util.c             |   20 +++++++++++++++
 lib/libalpm/util.h             |    1 +
 test/pacman/pmrule.py          |    2 +-
 test/pacman/tests/remove071.py |   33 +++++++++++++++++++++++++
 test/pacman/tests/sync600.py   |   51 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 122 insertions(+), 16 deletions(-)
 create mode 100644 test/pacman/tests/remove071.py
 create mode 100644 test/pacman/tests/sync600.py

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 73b533f..5d136c9 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -500,7 +500,7 @@ static char *get_pkgpath(alpm_db_t *db, alpm_pkg_t *info)
 
 #define READ_NEXT() do { \
 	if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \
-	_alpm_strtrim(line); \
+	_alpm_strip_newline(line); \
 } while(0)
 
 #define READ_AND_STORE(f) do { \
@@ -510,8 +510,8 @@ static char *get_pkgpath(alpm_db_t *db, alpm_pkg_t *info)
 
 #define READ_AND_STORE_ALL(f) do { \
 	char *linedup; \
-	READ_NEXT(); \
-	if(strlen(line) == 0) break; \
+	if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \
+	if(_alpm_strip_newline(line) == 0) break; \
 	STRDUP(linedup, line, goto error); \
 	f = alpm_list_add(f, linedup); \
 } while(1) /* note the while(1) and not (0) */
@@ -637,12 +637,12 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq)
 			goto error;
 		}
 		while(fgets(line, sizeof(line), fp)) {
-			_alpm_strtrim(line);
+			_alpm_strip_newline(line);
 			if(strcmp(line, "%FILES%") == 0) {
 				size_t files_count = 0, files_size = 0;
 				alpm_file_t *files = NULL;
 
-				while(fgets(line, sizeof(line), fp) && strlen(_alpm_strtrim(line))) {
+				while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line)) {
 					if(files_count >= files_size) {
 						size_t old_size = files_size;
 						if(files_size == 0) {
@@ -669,7 +669,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq)
 				info->files.count = files_count;
 				info->files.files = files;
 			} else if(strcmp(line, "%BACKUP%") == 0) {
-				while(fgets(line, sizeof(line), fp) && strlen(_alpm_strtrim(line))) {
+				while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line)) {
 					alpm_backup_t *backup;
 					CALLOC(backup, 1, sizeof(alpm_backup_t), goto error);
 					if(_alpm_split_backup(line, &backup)) {
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index 550f303..f80790c 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -149,13 +149,13 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t *
 
 	/* loop until we reach EOF or other error */
 	while((ret = _alpm_archive_fgets(a, &buf)) == ARCHIVE_OK) {
-		char *line = _alpm_strtrim(buf.line);
+		size_t len = _alpm_strip_newline(buf.line);
 
 		linenum++;
-		if(strlen(line) == 0 || line[0] == '#') {
+		if(len == 0 || buf.line[0] == '#') {
 			continue;
 		}
-		ptr = line;
+		ptr = buf.line;
 		key = strsep(&ptr, "=");
 		if(key == NULL || ptr == NULL) {
 			_alpm_log(handle, ALPM_LOG_DEBUG, "%s: syntax error in description file line %d\n",
@@ -213,7 +213,6 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t *
 									newpkg->name ? newpkg->name : "error", key, linenum);
 			}
 		}
-		line[0] = '\0';
 	}
 	if(ret != ARCHIVE_EOF) {
 		_alpm_log(handle, ALPM_LOG_DEBUG, "error parsing package descfile\n");
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 07356f0..d4c71a8 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -443,7 +443,8 @@ static int sync_db_populate(alpm_db_t *db)
 
 #define READ_NEXT() do { \
 	if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \
-	line = _alpm_strtrim(buf.line); \
+	line = buf.line; \
+	_alpm_strip_newline(line); \
 } while(0)
 
 #define READ_AND_STORE(f) do { \
@@ -453,9 +454,9 @@ static int sync_db_populate(alpm_db_t *db)
 
 #define READ_AND_STORE_ALL(f) do { \
 	char *linedup; \
-	READ_NEXT(); \
-	if(strlen(line) == 0) break; \
-	STRDUP(linedup, line, goto error); \
+	if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \
+	if(_alpm_strip_newline(buf.line) == 0) break; \
+	STRDUP(linedup, buf.line, goto error); \
 	f = alpm_list_add(f, linedup); \
 } while(1) /* note the while(1) and not (0) */
 
@@ -493,7 +494,8 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive,
 			|| strcmp(filename, "deltas") == 0) {
 		int ret;
 		while((ret = _alpm_archive_fgets(archive, &buf)) == ARCHIVE_OK) {
-			char *line = _alpm_strtrim(buf.line);
+			char *line = buf.line;
+			_alpm_strip_newline(line);
 
 			if(strcmp(line, "%NAME%") == 0) {
 				READ_NEXT();
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index 030cf43..123cd24 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -207,6 +207,26 @@ char *_alpm_strtrim(char *str)
 	return str;
 }
 
+/**
+ * Trim trailing newline from a string (if one exists).
+ * @param str a single line of text
+ * @return the length of the trimmed string
+ */
+size_t _alpm_strip_newline(char *str)
+{
+	size_t len;
+	if(str == '\0') {
+		return 0;
+	}
+	len = strlen(str);
+	while(str[len - 1] == '\n') {
+		len--;
+	}
+	str[len] = '\0';
+
+	return len;
+}
+
 /* Compression functions */
 
 /**
diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
index a75c5aa..9ee6370 100644
--- a/lib/libalpm/util.h
+++ b/lib/libalpm/util.h
@@ -95,6 +95,7 @@ int _alpm_makepath(const char *path);
 int _alpm_makepath_mode(const char *path, mode_t mode);
 int _alpm_copyfile(const char *src, const char *dest);
 char *_alpm_strtrim(char *str);
+size_t _alpm_strip_newline(char *str);
 int _alpm_unpack_single(alpm_handle_t *handle, const char *archive,
 		const char *prefix, const char *filename);
 int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix,
diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py
index cb7ae88..d64f519 100644
--- a/test/pacman/pmrule.py
+++ b/test/pacman/pmrule.py
@@ -42,7 +42,7 @@ def check(self, test):
         [testname, args] = self.rule.split("=")
         if testname[0] == "!":
             self.false = 1
-            testname = testname.lstrip("!")
+            testname = testname[1:]
         [kind, case] = testname.split("_")
         if "|" in args:
             [key, value] = args.split("|", 1)
diff --git a/test/pacman/tests/remove071.py b/test/pacman/tests/remove071.py
new file mode 100644
index 0000000..eff70a4
--- /dev/null
+++ b/test/pacman/tests/remove071.py
@@ -0,0 +1,33 @@
+# coding=utf8
+self.description = "Remove packages with evil filenames"
+
+self.filesystem = ["usr/bin/endwithspace",
+                   "spaces/name"]
+
+p1 = pmpkg("spaces")
+p1.files = ["usr/bin/endwithspace ",
+            " spaces/name"]
+self.addpkg2db("local", p1)
+
+p2 = pmpkg("unicodechars")
+# somewhat derived from FS#9906
+p2.files = ["usr/share/Märchen",
+            "usr/share/ƏƐƕƺ",
+            "usr/share/предупреждение",
+            "usr/share/סֶאבױ",
+            "usr/share/←↯↻⇈",
+            "usr/share/アヅヨヾ",
+            "usr/share/错误"]
+self.addpkg2db("local", p2)
+
+self.args = "-R %s %s" % (p1.name, p2.name)
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("!PKG_EXIST=%s" % p1.name)
+self.addrule("!PKG_EXIST=%s" % p2.name)
+
+for f in p1.files:
+    self.addrule("!FILE_EXIST=%s" % f)
+    self.addrule("FILE_EXIST=%s" % f.strip())
+for f in p2.files:
+    self.addrule("!FILE_EXIST=%s" % f)
diff --git a/test/pacman/tests/sync600.py b/test/pacman/tests/sync600.py
new file mode 100644
index 0000000..e0be668
--- /dev/null
+++ b/test/pacman/tests/sync600.py
@@ -0,0 +1,51 @@
+# coding=utf8
+self.description = "Sync packages with evil filenames"
+
+self.filesystem = ["usr/bin/endwithspace",
+                   "usr/bin/newendwithspace",
+                   "usr/bin/disappear",
+                   "spaces/name",
+                   "spaces/name2"]
+
+p1 = pmpkg("spaces")
+p1.files = ["usr/bin/endwithspace ",
+            "usr/bin/disappear ",
+            " spaces/name",
+            " spaces/gone"]
+self.addpkg2db("local", p1)
+
+sp1 = pmpkg("spaces", "1.1-1")
+sp1.files = ["usr/bin/endwithspace ",
+             "usr/bin/newendwithspace ",
+             " spaces/name",
+             " spaces/name2"]
+self.addpkg2db("sync", sp1)
+
+names = ["Märchen", "ƏƐƕƺ", "предупреждение", "סֶאבױ",
+         "←↯↻⇈", "アヅヨヾ", "错误"]
+
+p2 = pmpkg("unicodechars")
+# somewhat derived from FS#9906
+p2.files = ["usr/share/%s" % name for name in names]
+self.addpkg2db("local", p2)
+
+sp2 = pmpkg("unicodechars", "2.0-1")
+sp2.files = ["usr/man/%s" % name for name in names]
+self.addpkg2db("sync", sp2)
+
+self.args = "-S %s %s" % (sp1.name, sp2.name)
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("PKG_VERSION=%s|%s" % (sp1.name, sp1.version))
+self.addrule("PKG_VERSION=%s|%s" % (sp2.name, sp2.version))
+
+for f in self.filesystem:
+    self.addrule("FILE_EXIST=%s" % f)
+self.addrule("FILE_EXIST=usr/bin/endwithspace ")
+self.addrule("FILE_EXIST= spaces/name")
+self.addrule("FILE_EXIST= spaces/name2")
+self.addrule("!FILE_EXIST=usr/bin/disappear ")
+for f in p2.files:
+    self.addrule("!FILE_EXIST=%s" % f)
+for f in sp2.files:
+    self.addrule("FILE_EXIST=%s" % f)
-- 
1.7.6

    