Radish alpha
H
rad:z3QDZAW2FAfuLvihrhiyDC9fAD8G9
HardenedBSD Package Manager
Radicle
Git
Rework the keyword parsing function to make them more testable
Baptiste Daroussin committed 5 years ago
commit 6569097edf569aa051108f652307f5d6da1f0205
parent a639ddc
4 files changed +182 -57
modified libpkg/pkg_ports.c
@@ -173,8 +173,7 @@ parse_mode(const char *str)
	return (setmode(str));
}

-

-
static void
+
void
free_file_attr(struct file_attr *a)
{
	if (a == NULL)
@@ -1101,7 +1100,7 @@ external_keyword(struct plist *plist, char *keyword, char *line, struct file_att
	return (ret);
}

-
static struct file_attr *
+
 struct file_attr *
parse_keyword_args(char *args, char *keyword)
{
	struct file_attr *attr;
@@ -1117,10 +1116,10 @@ parse_keyword_args(char *args, char *keyword)
	do {
		args[0] = '\0';
		args++;
-
		if (*args == '\0')
-
			break;
		while (isspace(*args))
			args++;
+
		if (*args == '\0')
+
			break;
		if (owner == NULL) {
			owner = args;
		} else if (group == NULL) {
@@ -1131,9 +1130,6 @@ parse_keyword_args(char *args, char *keyword)
			fflags = args;
			break;
		} else {
-
			pkg_emit_error("Malformed keyword '%s', expecting "
-
			    "keyword or keyword(owner,group,mode,fflags...)",
-
			    keyword);
			return (NULL);
		}
	} while ((args = strchr(args, ',')) != NULL);
@@ -1158,12 +1154,14 @@ parse_keyword_args(char *args, char *keyword)
			return (NULL);
		}
	}
+
	if (owner == NULL && group == NULL && set == NULL)
+
		return (NULL);

	attr = xcalloc(1, sizeof(struct file_attr));
	if (owner != NULL && *owner != '\0')
-
		attr->owner = xstrdup(owner);
+
		attr->owner = xstrdup(rtrimspace(owner));
	if (group != NULL && *group != '\0')
-
		attr->group = xstrdup(group);
+
		attr->group = xstrdup(rtrimspace(group));
	if (set != NULL) {
		attr->mode = getmode(set, 0);
		free(set);
@@ -1174,27 +1172,13 @@ parse_keyword_args(char *args, char *keyword)
}

static int
-
parse_keywords(struct plist *plist, char *keyword, char *line)
+
parse_keywords(struct plist *plist, char *keyword,
+
    char *line, struct file_attr *attr)
{
	struct keyword *k;
	struct action *a;
-
	struct file_attr *attr = NULL;
-
	char *tmp;
	int ret = EPKG_FATAL;

-
	if ((tmp = strchr(keyword, '(')) != NULL &&
-
	    keyword[strlen(keyword) -1] != ')') {
-
		pkg_emit_error("Malformed keyword %s, expecting @keyword "
-
		    "or @keyword(owner,group,mode)", keyword);
-
		return (ret);
-
	}
-

-
	if (tmp != NULL) {
-
		attr = parse_keyword_args(tmp, keyword);
-
		if (attr == NULL)
-
			return (ret);
-
	}
-

	/* if keyword is empty consider it as a file */
	if (*keyword == '\0')
		return (file(plist, line, attr));
@@ -1204,9 +1188,9 @@ parse_keywords(struct plist *plist, char *keyword, char *line)
		LL_FOREACH(k->actions, a) {
			ret = a->perform(plist, line, attr);
			if (ret != EPKG_OK)
-
				goto end;
+
				break;
		}
-
		goto end;
+
		return (ret);
	}

	/*
@@ -1214,10 +1198,45 @@ parse_keywords(struct plist *plist, char *keyword, char *line)
	 * maybe it is defined externally
	 * let's try to find it
	 */
-
	ret = external_keyword(plist, keyword, line, attr);
-
end:
-
	free_file_attr(attr);
-
	return (ret);
+
	return (external_keyword(plist, keyword, line, attr));
+
}
+

+
char *
+
extract_keywords(char *line, char **keyword, struct file_attr **attr)
+
{
+
	char *k, *buf, *tmp;
+
	struct file_attr *a = NULL;
+

+
	buf = k = line;
+
	while (!(isspace(buf[0]) || buf[0] == '\0')) {
+
		if (buf[0] == '(' && (buf = strchr(buf, ')')) == NULL)
+
			return (NULL);
+
		buf++;
+
	}
+
	if (buf[0] != '\0') {
+
		buf[0] = '\0';
+
		buf++;
+
	}
+

+
	/* trim spaces after the keyword */
+
	while (isspace(buf[0]))
+
		buf++;
+

+
	pkg_debug(1, "Parsing plist, found keyword: '%s", k);
+

+
	if ((tmp = strchr(k, '(')) != NULL && k[strlen(k) -1] != ')')
+
		return (NULL);
+

+
	if (tmp != NULL) {
+
		a = parse_keyword_args(tmp, k);
+
		if (a == NULL)
+
			return (NULL);
+
	}
+

+
	*attr = a;
+
	*keyword = k;
+

+
	return (buf);
}

static void
@@ -1232,7 +1251,8 @@ flush_script_buffer(xstring *buf, struct pkg *p, int type)
int
plist_parse_line(struct plist *plist, char *line)
{
-
	char *keyword, *buf;
+
	char *keyword, *buf, *bkpline;
+
	struct file_attr *a;

	if (plist->ignore_next) {
		plist->ignore_next = false;
@@ -1243,32 +1263,19 @@ plist_parse_line(struct plist *plist, char *line)
		return (EPKG_OK);

	pkg_debug(1, "Parsing plist line: '%s'", line);
+
	bkpline = xstrdup(line);

	if (line[0] == '@') {
-
		keyword = line;
-
		keyword++; /* skip the @ */
-
		buf = keyword;
-
		while (!(isspace(buf[0]) || buf[0] == '\0')) {
-
			if (buf[0] == '(') {
-
				if ((buf = strchr(buf, ')')) == NULL) {
-
					pkg_emit_error("Malformed keyword %s, expecting @keyword "
-
				    "or @keyword(owner,group,mode)", keyword);
-
					return (EPKG_FATAL);
-
				}
-
			}
-
			buf++;
-
		}
-

-
		if (buf[0] != '\0') {
-
			buf[0] = '\0';
-
			buf++;
+
		keyword = NULL;
+
		a = NULL;
+
		buf = extract_keywords(line + 1, &keyword, &a);
+
		if (buf == NULL) {
+
			pkg_emit_error("Malformed keyword %s, expecting @keyword "
+
			    "or @keyword(owner,group,mode)", bkpline);
+
			return (EPKG_FATAL);
		}
-
		/* trim spaces */
-
		while (isspace(buf[0]))
-
			buf++;
-
		pkg_debug(1, "Parsing plist, found keyword: '%s", keyword);

-
		switch (parse_keywords(plist, keyword, buf)) {
+
		switch (parse_keywords(plist, keyword, buf, a)) {
		case EPKG_UNKNOWN:
			pkg_emit_error("unknown keyword %s: %s",
			    keyword, line);
modified libpkg/private/pkg.h
@@ -861,8 +861,11 @@ int pkg_open_root_fd(struct pkg *pkg);
void pkg_add_dir_to_del(struct pkg *pkg, const char *file, const char *dir);
struct plist *plist_new(struct pkg *p, const char *stage);
int plist_parse_line(struct plist *p, char *line);
+
char *extract_keywords(char *line, char **keyword, struct file_attr **attr);
+
struct file_attr *parse_keyword_args(char *args, char *keyword);
void plist_free(struct plist *);
int pkg_appendscript(struct pkg *pkg, const char *cmd, pkg_script type);
+
void free_file_attr(struct file_attr *a);

int pkg_add_lua_script(struct pkg *pkg, const char *data, pkg_lua_script type);
int pkg_addscript(struct pkg *pkg, const char *data, pkg_script type);
modified tests/frontend/create.sh
@@ -264,7 +264,7 @@ create_from_plist_bad_fflags_body() {

	atf_check \
		-o empty \
-
		-e inline:"${PROGNAME}: Malformed keyword '', wrong fflags\n" \
+
		-e inline:"${PROGNAME}: Malformed keyword '', wrong fflags\n${PROGNAME}: Malformed keyword @(,,,schg,bad) file1, expecting @keyword or @keyword(owner,group,mode)\n" \
		-s exit:1 \
		pkg create -o ${TMPDIR} -m . -p test.plist -r .
}
modified tests/lib/plist.c
@@ -1,5 +1,5 @@
/*-
-
 * Copyright (c) 2013 Baptiste Daroussin <bapt@FreeBSD.org>
+
 * Copyright (c) 2013-2020 Baptiste Daroussin <bapt@FreeBSD.org>
 * All rights reserved.
 *~
 * Redistribution and use in source and binary forms, with or without
@@ -57,6 +57,118 @@ ATF_TC_BODY(parse_mode, tc)
	free(set);
}

+
ATF_TC(parse_keyword_attributes);
+

+
ATF_TC_HEAD(parse_keyword_attributes, tc)
+
{
+
	atf_tc_set_md_var(tc, "descr",
+
	    "parse_keyword_attributes()");
+
}
+

+
ATF_TC_BODY(parse_keyword_attributes, tc)
+
{
+
	char buf[BUFSIZ];
+
	struct file_attr *a;
+

+
	strlcpy(buf, "()", BUFSIZ);
+
	ATF_REQUIRE(parse_keyword_args(buf, "plop") == NULL);
+

+
	strlcpy(buf, "(root, wheel)", BUFSIZ);
+
	ATF_REQUIRE((a = parse_keyword_args(buf, "plop")) != NULL);
+
	ATF_REQUIRE_STREQ(a->owner, "root");
+
	ATF_REQUIRE_STREQ(a->group, "wheel");
+
	free_file_attr(a);
+

+
	strlcpy(buf, "(root, wheel, 0755)", BUFSIZ);
+
	ATF_REQUIRE((a = parse_keyword_args(buf, "plop")) != NULL);
+
	ATF_REQUIRE_STREQ(a->owner, "root");
+
	ATF_REQUIRE_STREQ(a->group, "wheel");
+
	free_file_attr(a);
+

+
	strlcpy(buf, "(root, wheel, 0755,)", BUFSIZ);
+
	ATF_REQUIRE((a = parse_keyword_args(buf, "plop")) != NULL);
+
	ATF_REQUIRE_STREQ(a->owner, "root");
+
	ATF_REQUIRE_STREQ(a->group, "wheel");
+
	free_file_attr(a);
+
}
+

+
ATF_TC(parse_keyword);
+

+
ATF_TC_HEAD(parse_keyword, tc)
+
{
+
	atf_tc_set_md_var(tc, "descr",
+
	    "parse_keyword()");
+
}
+

+
ATF_TC_BODY(parse_keyword, tc)
+
{
+
	char *keyword;
+
	struct file_attr *attr;
+
	char buf[BUFSIZ];
+

+
	strlcpy(buf, "something", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_STREQ(extract_keywords(buf, &keyword, &attr), "");
+
	ATF_REQUIRE_STREQ(keyword, "something");
+
	ATF_REQUIRE_EQ(attr, NULL);
+

+
	/* empty keyword */
+
	strlcpy(buf, "", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_STREQ(extract_keywords(buf, &keyword, &attr), "");
+
	ATF_REQUIRE_STREQ(keyword, "");
+
	ATF_REQUIRE_EQ(attr, NULL);
+

+
	/* bad keyword */
+
	strlcpy(buf, "(", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_EQ(extract_keywords(buf, &keyword, &attr), NULL);
+
	ATF_REQUIRE_EQ(keyword, NULL);
+
	ATF_REQUIRE_EQ(attr, NULL);
+

+
	/* bad: empty keyword */
+
	strlcpy(buf, "()", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_EQ(extract_keywords(buf, &keyword, &attr), NULL);
+
	ATF_REQUIRE_EQ(keyword, NULL);
+
	ATF_REQUIRE_EQ(attr, NULL);
+

+
	/* ok only user keyword */
+
	strlcpy(buf, "(root) that", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_STREQ(extract_keywords(buf, &keyword, &attr), "that");
+
	ATF_REQUIRE_STREQ(keyword, "");
+
	ATF_REQUIRE(attr != NULL);
+
	ATF_REQUIRE_STREQ(attr->owner, "root");
+

+
	/* ok only group keyword */
+
	strlcpy(buf, "(,wheel) that", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_STREQ(extract_keywords(buf, &keyword, &attr), "that");
+
	ATF_REQUIRE_STREQ(keyword, "");
+
	ATF_REQUIRE(attr != NULL);
+
	ATF_REQUIRE_STREQ(attr->group, "wheel");
+

+
	/* ok only group with space keyword */
+
	strlcpy(buf, "( , wheel ,) that", BUFSIZ);
+
	keyword = NULL;
+
	attr = NULL;
+
	ATF_REQUIRE_STREQ(extract_keywords(buf, &keyword, &attr), "that");
+
	ATF_REQUIRE_STREQ(keyword, "");
+
	ATF_REQUIRE(attr != NULL);
+
	ATF_REQUIRE_STREQ(attr->group, "wheel");
+
	ATF_REQUIRE_EQ(attr->owner, NULL);
+

+
	strlcpy(buf, "(, wheel ,perm,ffags,)", BUFSIZ);
+
	ATF_REQUIRE_EQ(parse_keyword_args(buf, "plop"), NULL);
+
}
+

ATF_TC(parse_plist);

ATF_TC_HEAD(parse_plist, tc)
@@ -145,6 +257,7 @@ ATF_TC_BODY(parse_plist, tc)
	ATF_REQUIRE_EQ(EPKG_FATAL, plist_parse_line(plist, buf));

	strlcpy(buf, "@dirrm nonexisting", BUFSIZ);
+
	ATF_REQUIRE_EQ(EPKG_FATAL, plist_parse_line(plist, buf));

	pkg_free(p);
	plist_free(plist);
@@ -154,6 +267,8 @@ ATF_TP_ADD_TCS(tp)
{
	ATF_TP_ADD_TC(tp, parse_mode);
	ATF_TP_ADD_TC(tp, parse_plist);
+
	ATF_TP_ADD_TC(tp, parse_keyword_attributes);
+
	ATF_TP_ADD_TC(tp, parse_keyword);

	return (atf_no_error());
}