Handle escaped string literals (like "\tfoo\n") more correctly. (#1073)

"Unescape" all string literals in .osl and .oso as they are parsed,
then "escape" them properly for output.  We weren't being careful and
consistent before. This led to at least two subtle behavioral errors:

1. oslinfo would appear to add an extra set of escape characters to
   metadata values that contained escape codes.

2. If s = "\n", then strlen(s) was 1 if s was a local variable, but was
   incorrectly 2 (wowza!) if s was a parameter!

* Add fixed version of parse_string (AcademySoftwareFoundation/OpenImageIO#2386)

src/include/osl_pvt.h

@@ -710,7 +710,10 @@ class Symbol {
         return ((const ustring *)data())[index];
     }
 
-    /// Stream output
+    // Stream output. Note that print/print_vals assume that any string
+    // values are "raw" and they will be converted to C source code "escaped
+    // string" notation for printing. For example, a newline characer will
+    // be rendered into the stream as the two character sequence '\n'.
     std::ostream& print (std::ostream& out, int maxvals=100000000) const;
     std::ostream& print_vals (std::ostream& out, int maxvals=100000000) const;
 

src/liboslcomp/oslcomp.cpp

@@ -757,7 +757,8 @@ OSLCompilerImpl::write_oso_const_value (const ConstantSymbol *sym) const
     int nelements = std::max (1, type.arraylen);
     if (elemtype == TypeDesc::STRING)
         for (int i = 0;  i < nelements;  ++i)
-            oso ("\"%s\"%s", sym->strval(i), nelements>1 ? " " : "");
+            oso ("\"%s\"%s", OIIO::Strutil::escape_chars(sym->strval(i)),
+                 nelements>1 ? " " : "");
     else if (elemtype == TypeDesc::INT)
         for (int i = 0;  i < nelements;  ++i)
             oso ("%d%s", sym->intval(i), nelements>1 ? " " : "");

src/liboslcomp/osllex.l

@@ -242,9 +242,16 @@ void preprocess (const char *yytext);
 
 {STR}                   {
                             // grab the material between the quotes
-                            ustring s (yytext, 1, yyleng-2);
-                            yylval.s = s.c_str();
+                            string_view s(yytext + 1, yyleng - 2);
+                            std::string unescaped;
+                            if (s.find('\\') != string_view::npos) {
+                                // Only make a new string if we must unescape
+                                unescaped = OIIO::Strutil::unescape_chars(s);
+                                s = string_view(unescaped);
+                            }
+                            yylval.s = ustring(s).c_str();
                             SETLINE;
+                            // std::cerr << "osllex string '" << yylval.s << "'\n";
                             return STRING_LITERAL;
                         }
 

src/liboslcomp/symtab.cpp

@@ -121,7 +121,7 @@ Symbol::print_vals (std::ostream &out, int maxvals) const
     } else if (t.basetype == TypeDesc::STRING) {
         for (int j = 0;  j < n;  ++j)
             out << (j ? " " : "") << "\""
-                << Strutil::escape_chars(((ustring *)data())[j].string())
+                << Strutil::escape_chars(((ustring *)data())[j])
                 << "\"";
     }
     if (int(t.aggregate * t.numelements()) > maxvals)

src/liboslexec/osogram.y

@@ -258,16 +258,7 @@ initial_value
                 }
         | STRING_LITERAL
                 {
-                    string_view s ($1);
-                    // remove the quotes
-                    s.remove_prefix(1); s.remove_suffix(1);
-                    std::string unescaped;
-                    if (s.find('\\') != string_view::npos) {
-                        // Only make a new string if we must unescape
-                        unescaped = OIIO::Strutil::unescape_chars(s);
-                        s = string_view(unescaped);
-                    }
-                    OSOReader::osoreader->symdefault (s.c_str());
+                    OSOReader::osoreader->symdefault ($1);
                     $$ = 0;
                 }
         ;

src/liboslexec/osolex.l

@@ -74,7 +74,7 @@ FLT2            [-+]?{DIGIT}*\.{DIGIT}+{E}?
 FLT3            [-+]?{DIGIT}+{E}
 FLT             {FLT1}|{FLT2}|{FLT3}
  /* string literal */
-STR     \"(\\.|[^\\"])*\"
+STR     \"(\\.|[^\\"\n])*\"
  /* Identifier: alphanumeric, may contain digits after the first character.
   * Also '$' and '.' are allowed!
   */
@@ -217,9 +217,16 @@ using namespace OSL::pvt;
                         }
 
 {STR}                   {
-                            ustring s (yytext, yyleng);
-                            yylval.s = s.c_str();
-                            // std::cerr << "lex string '" << yylval.s << "'\n";
+                            // grab the material between the quotes
+                            string_view s(yytext + 1, yyleng - 2);
+                            std::string unescaped;
+                            if (s.find('\\') != string_view::npos) {
+                                // Only make a new string if we must unescape
+                                unescaped = OIIO::Strutil::unescape_chars(s);
+                                s = string_view(unescaped);
+                            }
+                            yylval.s = ustring(s).c_str();
+                            // std::cerr << "osolex string '" << yylval.s << "'\n";
                             return STRING_LITERAL;
                         }
 

src/liboslquery/oslquery.cpp

@@ -180,6 +180,52 @@ OSOReaderQuery::parameter_done ()
 
 
 
+
+#if OPENIMAGEIO_VERSION < 20109
+// A bug present in Strutil::parse_string until OIIO 2.1.8 caused incorrect
+// parsing of hints with embedded escaped quote characters. Supply a copy
+// of the fixed version here until our minimum OIIO is new enough to be
+// sure that the bug is fixed.
+static bool
+parse_string(string_view& str, string_view& val, bool eat=true,
+             OIIO::Strutil::QuoteBehavior keep_quotes=OIIO::Strutil::DeleteQuotes) noexcept
+{
+    using namespace OIIO::Strutil;
+    string_view p = str;
+    skip_whitespace(p);
+    if (str.empty())
+        return false;
+    char lead_char    = p.front();
+    bool quoted       = parse_char(p, '\"') || parse_char(p, '\'');
+    const char *begin = p.begin(), *end = p.begin();
+    bool escaped = false;  // was the prior character a backslash
+    while (end != p.end()) {
+        if (isspace(*end) && !quoted)
+            break;  // not quoted and we hit whitespace: we're done
+        if (quoted && *end == lead_char && !escaped)
+            break;  // closing quote -- we're done (beware embedded quote)
+        escaped = (end[0] == '\\') && (!escaped);
+        ++end;
+    }
+    if (quoted && keep_quotes == KeepQuotes) {
+        if (*end == lead_char)
+            val = string_view(begin - 1, size_t(end - begin) + 2);
+        else
+            val = string_view(begin - 1, size_t(end - begin) + 1);
+    } else {
+        val = string_view(begin, size_t(end - begin));
+    }
+    p.remove_prefix(size_t(end - begin));
+    if (quoted && p.size() && p[0] == lead_char)
+        p.remove_prefix(1);  // eat closing quote
+    if (eat)
+        str = p;
+    return quoted || val.size();
+}
+#endif
+
+
+
 void
 OSOReaderQuery::hint (string_view hintstring)
 {
@@ -195,11 +241,14 @@ OSOReaderQuery::hint (string_view hintstring)
         // std::cerr << "    " << name << " : " << type << "\n";
         OSLQuery::Parameter p;
         p.name = name;
-        p.type = TypeDesc (type.c_str());
+        p.type = TypeDesc (type);
         if (p.type.basetype == TypeDesc::STRING) {
             string_view val;
-            while (Strutil::parse_string (hintstring, val)) {
-                p.sdefault.emplace_back(val);
+#if OPENIMAGEIO_VERSION >= 20109
+            using Strutil::parse_string;
+#endif
+            while (parse_string (hintstring, val)) {
+                p.sdefault.emplace_back(OIIO::Strutil::unescape_chars(val));
                 if (Strutil::parse_char (hintstring, '}'))
                     break;
                 Strutil::parse_char (hintstring, ',');

src/oslinfo/oslinfo.cpp

@@ -80,10 +80,12 @@ print_default_string_vals (const OSLQuery::Parameter *p, bool verbose)
         ne = p->type.numelements();
     if (verbose) {
         for (size_t a = 0;  a < ne;  ++a)
-            std::cout << "\t\tDefault value: \"" << p->sdefault[a] << "\"\n";
+            std::cout << "\t\tDefault value: \""
+                      << OIIO::Strutil::escape_chars(p->sdefault[a]) << "\"\n";
     } else {
         for (size_t a = 0;  a < ne;  ++a)
-            std::cout << "\"" << p->sdefault[a] << "\" ";
+            std::cout << "\"" << OIIO::Strutil::escape_chars(p->sdefault[a])
+                      << "\" ";
         std::cout << "\n";
     }
 }

testsuite/oslinfo-metadata/metadata.osl

@@ -4,7 +4,9 @@ surface metadata
     int myparam1 = 1 [[ int i = 0, float f = 1.0, string s = "foo" ]],
     int myparam2 = 2 [[ string s[2] = { "foo", "bar" } ]],
     int myparam3 = 3 [[ float minmax[2] = { 42, 44 } ]],
-    int myparam4 = 4 [[ color c = color(1,2,3) ]]
+    int myparam4 = 4 [[ color c = color(1,2,3) ]],
+    int myparam5 = 5 [[ string s = "I have\n\"Escape\"\tsequences\n" ]]
     )
 {
+    string ss = "I have\n\"Escape\"\tsequences\n";
 }

testsuite/oslinfo-metadata/ref/out.txt

@@ -15,3 +15,6 @@ surface "metadata"
     "myparam4" "int"
 		Default value: 4
 		metadata: color c = 1 2 3
+    "myparam5" "int"
+		Default value: 5
+		metadata: string s = "I have\n\"Escape\"\tsequences\n"

testsuite/string/ref/out.txt

@@ -65,4 +65,5 @@ getchar("Hello World!", 12) = 0
 
 hash("Hello World!") = -16050688
 hash("") = 0
+strlen newline: param 1 local 1 literal 1
 

testsuite/string/test.osl

@@ -72,7 +72,7 @@ void test_split (string s, string sep)
 
 
 shader
-test ()
+test (string param_newline = "\n")
 {
     printf ("test string operations:\n\n");
 
@@ -81,11 +81,14 @@ test ()
             "foo" "bar" "baz");
 
     // Test escape sequences
+    string local_newline = "\n";
     printf ("\n");
     printf ("foo\\nbar\n");  // should print:  foo\bar
     printf ("blah '%s'\n", "foo\\nbar\n");  // should print: blah 'foo\bar
                                             //                    '
     printf ("blah2 '%s'\n", "foo\\\\");     // should print: blah2 'foo\\'
+    if (local_newline != param_newline)
+        error ("escaped strings don't match params vs locals\n");
 
     // Test format
     printf ("\n");
@@ -164,4 +167,9 @@ test ()
     printf("\n");
     printf("hash(\"%s\") = %d\n", a, hash(a));
     printf("hash(\"\") = %d\n", hash(""));
+
+    // Regression test for bugs with understanding length of strings having
+    // embedded escape sequences:
+    printf ("strlen newline: param %d local %d literal %d\n",
+            strlen(param_newline), strlen(local_newline), strlen("\n"));
 }