From: "J. Bruce Fields" <bfie...@redhat.com>

string_escape_mem is harder to use than necessary:

        - ESCAPE_NP sounds like it means "escape nonprinting
          characters", but actually means "do not escape printing
          characters"
        - the use of the "only" string to limit the list of escaped
          characters rather than supplement them is confusing and
          unehlpful.

So:

        - use the "only" string to add to, rather than replace, the list
          of characters to escape
        - separate flags into those that select which characters to
          escape, and those that choose the format of the escaping ("\ "
          vs "\x20" vs "\040".)  Make flags that select characters just
          select the union when they're OR'd together.

Untested.  The tests themselves, especially, I'm unsure about.

Signed-off-by: J. Bruce Fields <bfie...@redhat.com>
---
 fs/proc/array.c                |  2 +-
 fs/seq_file.c                  |  2 +-
 include/linux/string_helpers.h | 14 +++----
 lib/string_helpers.c           | 76 +++++++++++++++-------------------
 lib/test-string_helpers.c      | 41 ++++++++----------
 lib/vsprintf.c                 |  2 +-
 net/sunrpc/cache.c             |  2 +-
 7 files changed, 62 insertions(+), 77 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 982c95d09176..bdeeb3318fa2 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -111,7 +111,7 @@ void proc_task_name(struct seq_file *m, struct task_struct 
*p, bool escape)
        size = seq_get_buf(m, &buf);
        if (escape) {
                ret = string_escape_str(tcomm, buf, size,
-                                       ESCAPE_SPECIAL, "\n\\");
+                                       ESCAPE_STYLE_SLASH, "\n\\");
                if (ret >= size)
                        ret = -1;
        } else {
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 1600034a929b..63e5a7c4dbf7 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -379,7 +379,7 @@ void seq_escape(struct seq_file *m, const char *s, const 
char *esc)
        size_t size = seq_get_buf(m, &buf);
        int ret;
 
-       ret = string_escape_str(s, buf, size, ESCAPE_OCTAL, esc);
+       ret = string_escape_str(s, buf, size, ESCAPE_STYLE_OCTAL, esc);
        seq_commit(m, ret < size ? ret : -1);
 }
 EXPORT_SYMBOL(seq_escape);
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 7bf0d137373d..5d350f7f6874 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -41,13 +41,13 @@ static inline int string_unescape_any_inplace(char *buf)
        return string_unescape_any(buf, buf, 0);
 }
 
-#define ESCAPE_SPECIAL         0x02
-#define ESCAPE_OCTAL           0x08
-#define ESCAPE_ANY             (ESCAPE_OCTAL | ESCAPE_SPECIAL)
-#define ESCAPE_NP              0x10
-#define ESCAPE_ANY_NP          (ESCAPE_ANY | ESCAPE_NP)
-#define ESCAPE_HEX             0x20
-#define ESCAPE_FLAG_MASK       0x3a
+#define ESCAPE_SPECIAL         0x01
+#define ESCAPE_NP              0x02
+#define ESCAPE_ANY_NP          (ESCAPE_SPECIAL | ESCAPE_NP)
+#define ESCAPE_STYLE_SLASH     0x20
+#define ESCAPE_STYLE_OCTAL     0x40
+#define ESCAPE_STYLE_HEX       0x80
+#define ESCAPE_FLAG_MASK       0xe3
 
 int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
                unsigned int flags, const char *only);
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index ac72159d3980..47f40406f9d4 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -400,6 +400,11 @@ static bool escape_hex(unsigned char c, char **dst, char 
*end)
        return true;
 }
 
+static bool is_special(char c)
+{
+       return c == '\0' || strchr("\f\n\r\t\v\\\a\e", c);
+}
+
 /**
  * string_escape_mem - quote characters in the given memory buffer
  * @src:       source buffer (unescaped)
@@ -407,23 +412,18 @@ static bool escape_hex(unsigned char c, char **dst, char 
*end)
  * @dst:       destination buffer (escaped)
  * @osz:       destination buffer size
  * @flags:     combination of the flags
- * @only:      NULL-terminated string containing characters used to limit
- *             the selected escape class. If characters are included in @only
- *             that would not normally be escaped by the classes selected
- *             in @flags, they will be copied to @dst unescaped.
+ * @esc:       NULL-terminated string containing characters which
+ *             should also be escaped.
  *
- * Description:
- * The process of escaping byte buffer includes several parts. They are applied
- * in the following sequence.
  *
- *     1. The character is matched to the printable class, if asked, and in
- *        case of match it passes through to the output.
- *     2. The character is not matched to the one from @only string and thus
- *        must go as-is to the output.
- *     3. The character is checked if it falls into the class given by @flags.
- *        %ESCAPE_OCTAL and %ESCAPE_HEX are going last since they cover any
- *        character. Note that they actually can't go together, otherwise
- *        %ESCAPE_HEX will be ignored.
+ * Description:
+ * The characters selected by ESCAPE_* flags and by the "esc" string
+ * are escaped, using the escaping specified by the ESCAPE_STYLE_*
+ * flags.  If ESCAPE_STYLE_SLASH is specified together with one of the
+ * ESCAPE_STYLE_OCTAL or ESCAPE_STYLE_HEX flags, then those characters
+ * for which special slash escapes exist will be escaped using those,
+ * with others escaped using octal or hexidecimal values.  If
+ * unspecified, the default is ESCAPE_STYLE_OCTAL.
  *
  * Caller must provide valid source and destination pointers. Be aware that
  * destination buffer will not be NULL-terminated, thus caller have to append
@@ -439,14 +439,14 @@ static bool escape_hex(unsigned char c, char **dst, char 
*end)
  *             '\a' - alert (BEL)
  *             '\e' - escape
  *             '\0' - null
- *     %ESCAPE_OCTAL:
- *             '\NNN' - byte with octal value NNN (3 digits)
- *     %ESCAPE_ANY:
- *             all previous together
  *     %ESCAPE_NP:
  *             escape only non-printable characters (checked by isprint)
  *     %ESCAPE_ANY_NP:
  *             all previous together
+ *     %ESCAPE_STYLE_SLASH:
+ *             Escape using the slash codes shown above
+ *     %ESCAPE_STYLE_OCTAL:
+ *             '\NNN' - byte with octal value NNN (3 digits)
  *     %ESCAPE_HEX:
  *             '\xHH' - byte with hexadecimal value HH (2 digits)
  *
@@ -457,39 +457,31 @@ static bool escape_hex(unsigned char c, char **dst, char 
*end)
  * dst for a '\0' terminator if and only if ret < osz.
  */
 int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
-                     unsigned int flags, const char *only)
+                     unsigned int flags, const char *esc)
 {
        char *p = dst;
        char *end = p + osz;
-       bool is_dict = only && *only;
+       bool is_dict = esc && *esc;
 
        while (isz--) {
                unsigned char c = *src++;
 
-               /*
-                * Apply rules in the following sequence:
-                *      - the character is printable, when @flags has
-                *        %ESCAPE_NP bit set
-                *      - the @only string is supplied and does not contain a
-                *        character under question
-                *      - the character doesn't fall into a class of symbols
-                *        defined by given @flags
-                * In these cases we just pass through a character to the
-                * output buffer.
-                */
-               if ((flags & ESCAPE_NP && isprint(c)) ||
-                   (is_dict && !strchr(only, c))) {
-                       /* do nothing */
-               } else {
-                       if (flags & ESCAPE_SPECIAL && escape_special(c, &p, 
end))
-                               continue;
+               if ((is_dict && strchr(esc, c)) ||
+                   (flags & ESCAPE_NP && !isprint(c)) ||
+                   (flags & ESCAPE_SPECIAL && is_special(c))) {
 
-                       /* ESCAPE_OCTAL and ESCAPE_HEX always go last */
-                       if (flags & ESCAPE_OCTAL && escape_octal(c, &p, end))
+                       if (flags & ESCAPE_STYLE_SLASH &&
+                                       escape_special(c, &p, end))
                                continue;
 
-                       if (flags & ESCAPE_HEX && escape_hex(c, &p, end))
+                       if (flags & ESCAPE_STYLE_HEX &&
+                                       escape_hex(c, &p, end))
                                continue;
+                       /*
+                        * Always the default, so a caller doesn't
+                        * accidentally leave something unescaped:
+                        */
+                       escape_octal(c, &p, end);
                }
 
                escape_passthrough(c, &p, end);
@@ -526,7 +518,7 @@ char *kstrdup_quotable(const char *src, gfp_t gfp)
 {
        size_t slen, dlen;
        char *dst;
-       const int flags = ESCAPE_HEX;
+       const int flags = ESCAPE_STYLE_HEX;
        const char esc[] = "\f\n\r\t\v\a\e\\\"";
 
        if (!src)
diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
index 0da3c195a327..d40fedab24ad 100644
--- a/lib/test-string_helpers.c
+++ b/lib/test-string_helpers.c
@@ -127,13 +127,13 @@ static const struct test_string_2 escape0[] __initconst = 
{{
        .in = "\\h\\\"\a\e\\",
        .s1 = {{
                .out = "\\\\h\\\\\"\\a\\e\\\\",
-               .flags = ESCAPE_SPECIAL,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_SLASH,
        },{
                .out = "\\\\\\150\\\\\\042\\a\\e\\\\",
-               .flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_SLASH | 
ESCAPE_STYLE_OCTAL,
        },{
                .out = "\\\\\\x68\\\\\\x22\\a\\e\\\\",
-               .flags = ESCAPE_SPECIAL | ESCAPE_HEX,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_SLASH | ESCAPE_STYLE_HEX,
        },{
                /* terminator */
        }},
@@ -141,28 +141,19 @@ static const struct test_string_2 escape0[] __initconst = 
{{
        .in = "\eb \\C\007\"\x90\r]",
        .s1 = {{
                .out = "\\eb \\\\C\\a\"\x90\\r]",
-               .flags = ESCAPE_SPECIAL,
-       },{
-               .out = "\\033\\142\\040\\134\\103\\007\\042\\220\\015\\135",
-               .flags = ESCAPE_OCTAL,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_SLASH,
        },{
                .out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\r\\135",
-               .flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
-       },{
-               .out = "\eb \\C\007\"\x90\r]",
-               .flags = ESCAPE_NP,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_SLASH | 
ESCAPE_STYLE_OCTAL,
        },{
                .out = "\\eb \\C\\a\"\x90\\r]",
-               .flags = ESCAPE_SPECIAL | ESCAPE_NP,
-       },{
-               .out = "\\033b \\C\\007\"\\220\\015]",
-               .flags = ESCAPE_OCTAL | ESCAPE_NP,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_SLASH | 
ESCAPE_STYLE_OCTAL,
        },{
                .out = "\\eb \\C\\a\"\\220\\r]",
-               .flags = ESCAPE_SPECIAL ESCAPE_OCTAL | ESCAPE_NP,
+               .flags = ESCAPE_SPECIAL | ESCAPE_STYLE_OCTAL | ESCAPE_NP,
        },{
                .out = "\\x1bb \\C\\x07\"\\x90\\x0d]",
-               .flags = ESCAPE_NP | ESCAPE_HEX,
+               .flags = ESCAPE_NP | ESCAPE_STYLE_HEX,
        },{
                /* terminator */
        }},
@@ -175,10 +166,10 @@ static const struct test_string_2 escape1[] __initconst = 
{{
        .in = "\f\\ \n\r\t\v",
        .s1 = {{
                .out = "\f\\134\\040\n\\015\\011\v",
-               .flags = ESCAPE_OCTAL,
+               .flags = ESCAPE_STYLE_OCTAL,
        },{
                .out = "\f\\x5c\\x20\n\\x0d\\x09\v",
-               .flags = ESCAPE_HEX,
+               .flags = ESCAPE_STYLE_HEX,
        },{
                /* terminator */
        }},
@@ -186,7 +177,7 @@ static const struct test_string_2 escape1[] __initconst = {{
        .in = "\\h\\\"\a\e\\",
        .s1 = {{
                .out = "\\134h\\134\"\a\e\\134",
-               .flags = ESCAPE_OCTAL,
+               .flags = ESCAPE_STYLE_OCTAL,
        },{
                /* terminator */
        }},
@@ -194,7 +185,7 @@ static const struct test_string_2 escape1[] __initconst = {{
        .in = "\eb \\C\007\"\x90\r]",
        .s1 = {{
                .out = "\e\\142\\040\\134C\007\"\x90\\015]",
-               .flags = ESCAPE_OCTAL,
+               .flags = ESCAPE_STYLE_OCTAL,
        },{
                /* terminator */
        }},
@@ -211,9 +202,9 @@ static __init const char *test_string_find_match(const 
struct test_string_2 *s2,
        if (!flags)
                return s2->in;
 
-       /* ESCAPE_OCTAL has a higher priority */
-       if (flags & ESCAPE_OCTAL)
-               flags &= ~ESCAPE_HEX;
+       /* ESCAPE_OCTAL is default: */
+       if (!(flags & ESCAPE_STYLE_HEX))
+               flags |= ESCAPE_STYLE_OCTAL;
 
        for (i = 0; i < TEST_STRING_2_MAX_S1 && s1->out; i++, s1++)
                if (s1->flags == flags)
@@ -266,6 +257,8 @@ static __init void test_string_escape(const char *name,
                memcpy(&out_test[q_test], out, len);
                q_test += len;
        }
+       if (!p)
+               goto out;
 
        q_real = string_escape_mem(in, p, out_real, out_size, flags, esc);
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 5522d2a052e1..020aff0c3fd9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1553,7 +1553,7 @@ char *escaped_string(char *buf, char *end, u8 *addr, 
struct printf_spec spec,
         * had the buffer been big enough.
         */
        buf += string_escape_mem(addr, len, buf, buf < end ? end - buf : 0,
-                                ESCAPE_ANY_NP, NULL);
+                                ESCAPE_ANY_NP|ESCAPE_STYLE_SLASH, NULL);
 
        return buf;
 }
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 6f1528f271ee..1b5a2b9e7808 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1118,7 +1118,7 @@ void qword_add(char **bpp, int *lp, char *str)
 
        if (len < 0) return;
 
-       ret = string_escape_str(str, bp, len, ESCAPE_OCTAL, "\\ \n\t");
+       ret = string_escape_str(str, bp, len, ESCAPE_STYLE_OCTAL, "\\ \n\t");
        if (ret >= len) {
                bp += len;
                len = -1;
-- 
2.21.0

Reply via email to