summaryrefslogtreecommitdiff
blob: aaacdc2674c00c9c883383896db4e7ae12f5b623 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
From 4cca923b732990bec0c699b2e69911c2221b2498 Mon Sep 17 00:00:00 2001
From: David Lutterkort <lutter@watzmann.net>
Date: Fri, 4 Aug 2017 17:13:52 -0700
Subject: [PATCH] * src/pathx.c (parse_name): correctly handle trailing
 whitespace in names

When a name ended in whitespace, we incorrectly assumed it was always ok to
trim that whitespace. That is not true if that whitespace is escaped,
i.e. if the path expression is something like '/x\ '. In that case, the
name really needs to be literally 'x ', i.e., we can not trim that
whitespace.

The incorrect behavior led to turning '/x\ ' first into 'x\' and then,
because we assume that '\' is always followed by a character inside the
string, when we removed the escaping '\', we would read beyond the end of
the intermediate string result; if we were lucky, that would lead to a
crash, otherwise we'd continue with junk.

We now make sure that escaped whitespace at the end of a string does not
get stripped, avoiding all these headaches.

Fixes RHBZ https://bugzilla.redhat.com/show_bug.cgi?id=1475621
---
 src/pathx.c        | 27 +++++++++++++++++++------
 tests/test-xpath.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/src/pathx.c b/src/pathx.c
index d292cb30..9a2f9c76 100644
--- a/src/pathx.c
+++ b/src/pathx.c
@@ -1710,6 +1710,16 @@ int pathx_escape_name(const char *in, char **out) {
     return 0;
 }
 
+/* Return true if POS is preceded by an odd number of backslashes, i.e., if
+ * POS is escaped. Stop the search when we get to START */
+static bool backslash_escaped(const char *pos, const char *start) {
+    bool result=false;
+    while (pos-- > start && *pos == '\\') {
+        result = !result;
+    }
+    return result;
+}
+
 /*
  * NameNoWS ::= [^][|/\= \t\n] | \\.
  * NameWS   ::= [^][|/\=] | \\.
@@ -1719,11 +1729,14 @@ static char *parse_name(struct state *state) {
     const char *s = state->pos;
     char *result;
 
+    /* Advance state->pos until it points to the first character that is
+     * not part of a name. */
     while (*state->pos != '\0' && strchr(name_follow, *state->pos) == NULL) {
-        /* This is a hack: since we allow spaces in names, we need to avoid
-         * gobbling up stuff that is in follow(Name), e.g. 'or' so that
-         * things like [name1 or name2] still work.
-         */
+        /* Since we allow spaces in names, we need to avoid gobbling up
+         * stuff that is in follow(Name), e.g. 'or' so that things like
+         * [name1 or name2] still work. In other words, we'll parse 'x frob
+         * y' as one name, but for 'x or y', we consider 'x' a name in its
+         * own right. */
         if (STREQLEN(state->pos, " or ", strlen(" or ")) ||
             STREQLEN(state->pos, " and ", strlen(" and ")))
             break;
@@ -1738,10 +1751,12 @@ static char *parse_name(struct state *state) {
         state->pos += 1;
     }
 
-    /* Strip trailing white space */
+    /* Strip trailing white space. Make sure we respect escaped whitespace
+     * and don't strip it as in "x\\ " */
     if (state->pos > s) {
         state->pos -= 1;
-        while (isspace(*state->pos) && state->pos >= s)
+        while (isspace(*state->pos) && state->pos > s
+               && !backslash_escaped(state->pos, s))
             state->pos -= 1;
         state->pos += 1;
     }
diff --git a/tests/test-xpath.c b/tests/test-xpath.c
index 3e418e5f..82986474 100644
--- a/tests/test-xpath.c
+++ b/tests/test-xpath.c
@@ -355,6 +355,62 @@ static int test_wrong_regexp_flag(struct augeas *aug) {
     return -1;
 }
 
+static int test_trailing_ws_in_name(struct augeas *aug) {
+    int r;
+
+    printf("%-30s ... ", "trailing_ws_in_name");
+
+    /* We used to incorrectly lop escaped whitespace off the end of a
+     * name. Make sure that we really create a tree node with label 'x '
+     * with the below set, and look for it in a number of ways to ensure we
+     * are not lopping off trailing whitespace. */
+    r = aug_set(aug, "/ws\\ ", "1");
+    if (r < 0) {
+        fprintf(stderr, "failed to set '/ws ': %d\n", r);
+        goto fail;
+    }
+    /* We did not create a node with label 'ws' */
+    r = aug_get(aug, "/ws", NULL);
+    if (r != 0) {
+        fprintf(stderr, "created '/ws' instead: %d\n", r);
+        goto fail;
+    }
+
+    /* We did not create a node with label 'ws\t' (this also checks that we
+     * don't create something like 'ws\\' by dropping the last whitespace
+     * character. */
+    r = aug_get(aug, "/ws\\\t", NULL);
+    if (r != 0) {
+        fprintf(stderr, "found '/ws\\t': %d\n", r);
+        goto fail;
+    }
+
+    /* But we did create 'ws ' */
+    r = aug_get(aug, "/ws\\ ", NULL);
+    if (r != 1) {
+        fprintf(stderr, "could not find '/ws ': %d\n", r);
+        goto fail;
+    }
+
+    /* If the whitespace is preceded by an even number of '\\' chars,
+     * whitespace must be stripped */
+    r = aug_set(aug, "/nows\\\\ ", "1");
+    if (r < 0) {
+        fprintf(stderr, "set of '/nows' failed: %d\n", r);
+        goto fail;
+    }
+    r = aug_get(aug, "/nows\\\\", NULL);
+    if (r != 1) {
+        fprintf(stderr, "could not get '/nows\\'\n");
+        goto fail;
+    }
+    printf("PASS\n");
+    return 0;
+ fail:
+    printf("FAIL\n");
+    return -1;
+}
+
 static int run_tests(struct test *tests, int argc, char **argv) {
     char *lensdir;
     struct augeas *aug = NULL;
@@ -398,6 +454,9 @@ static int run_tests(struct test *tests, int argc, char **argv) {
 
         if (test_wrong_regexp_flag(aug) < 0)
             result = EXIT_FAILURE;
+
+        if (test_trailing_ws_in_name(aug) < 0)
+            result = EXIT_FAILURE;
     }
     aug_close(aug);
     free(lensdir);