Add extra error text in the property parser
authorRichard Levitte <levitte@openssl.org>
Wed, 19 Jun 2019 06:37:19 +0000 (08:37 +0200)
committerRichard Levitte <levitte@openssl.org>
Wed, 19 Jun 2019 09:25:27 +0000 (11:25 +0200)
With properties being specified in all kinds of places, including hard
coded in providers, it's not always easy to figure out exactly what
string was incorrect when the parser would just say something like
'parse failed' with no more details.

Adding extra data to the error, showing exactly what string is
incorrect, helps a bit.  At the very least, this gives anyone
interested something to grep for.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9190)

crypto/property/property_parse.c

index c63a433..0b4dcfc 100644 (file)
@@ -92,6 +92,7 @@ static int parse_name(OPENSSL_CTX *ctx, const char *t[], int create,
     for (;;) {
         if (!ossl_isalpha(*s)) {
             PROPerr(PROP_F_PARSE_NAME, PROP_R_NOT_AN_IDENTIFIER);
+            ERR_add_error_data(2, "HERE-->", *t);
             return 0;
         }
         do {
@@ -110,13 +111,14 @@ static int parse_name(OPENSSL_CTX *ctx, const char *t[], int create,
         s++;
     }
     name[i] = '\0';
-    *t = skip_space(s);
-    if (!err) {
-        *idx = ossl_property_name(ctx, name, user_name && create);
-        return 1;
+    if (err) {
+        PROPerr(PROP_F_PARSE_NAME, PROP_R_NAME_TOO_LONG);
+        ERR_add_error_data(2, "HERE-->", *t);
+        return 0;
     }
-    PROPerr(PROP_F_PARSE_NAME, PROP_R_NAME_TOO_LONG);
-    return 0;
+    *t = skip_space(s);
+    *idx = ossl_property_name(ctx, name, user_name && create);
+    return 1;
 }
 
 static int parse_number(const char *t[], PROPERTY_DEFINITION *res)
@@ -131,6 +133,7 @@ static int parse_number(const char *t[], PROPERTY_DEFINITION *res)
     } while (ossl_isdigit(*s));
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
         PROPerr(PROP_F_PARSE_NUMBER, PROP_R_NOT_A_DECIMAL_DIGIT);
+        ERR_add_error_data(2, "HERE-->", *t);
         return 0;
     }
     *t = skip_space(s);
@@ -155,6 +158,7 @@ static int parse_hex(const char *t[], PROPERTY_DEFINITION *res)
     } while (ossl_isxdigit(*++s));
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
         PROPerr(PROP_F_PARSE_HEX, PROP_R_NOT_AN_HEXADECIMAL_DIGIT);
+        ERR_add_error_data(2, "HERE-->", *t);
         return 0;
     }
     *t = skip_space(s);
@@ -175,6 +179,7 @@ static int parse_oct(const char *t[], PROPERTY_DEFINITION *res)
     } while (ossl_isdigit(*++s) && *s != '9' && *s != '8');
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
         PROPerr(PROP_F_PARSE_OCT, PROP_R_NOT_AN_OCTAL_DIGIT);
+        ERR_add_error_data(2, "HERE-->", *t);
         return 0;
     }
     *t = skip_space(s);
@@ -199,16 +204,22 @@ static int parse_string(OPENSSL_CTX *ctx, const char *t[], char delim,
         s++;
     }
     if (*s == '\0') {
+        char buf[2] = { 0, 0 };
+
         PROPerr(PROP_F_PARSE_STRING,
                 PROP_R_NO_MATCHING_STRING_DELIMETER);
+        buf[0] = delim;
+        ERR_add_error_data(3, "HERE-->", buf, *t);
         return 0;
     }
     v[i] = '\0';
-    *t = skip_space(s + 1);
-    if (err)
+    if (err) {
         PROPerr(PROP_F_PARSE_STRING, PROP_R_STRING_TOO_LONG);
-    else
+        ERR_add_error_data(2, "HERE-->", *t);
+    } else {
         res->v.str_val = ossl_property_value(ctx, v, create);
+    }
+    *t = skip_space(s + 1);
     res->type = PROPERTY_TYPE_STRING;
     return !err;
 }
@@ -232,14 +243,17 @@ static int parse_unquoted(OPENSSL_CTX *ctx, const char *t[],
     }
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
         PROPerr(PROP_F_PARSE_UNQUOTED, PROP_R_NOT_AN_ASCII_CHARACTER);
+        ERR_add_error_data(2, "HERE-->", s);
         return 0;
     }
     v[i] = 0;
-    *t = skip_space(s);
-    if (err)
+    if (err) {
         PROPerr(PROP_F_PARSE_UNQUOTED, PROP_R_STRING_TOO_LONG);
-    else
+        ERR_add_error_data(2, "HERE-->", *t);
+    } else {
         res->v.str_val = ossl_property_value(ctx, v, create);
+    }
+    *t = skip_space(s);
     res->type = PROPERTY_TYPE_STRING;
     return !err;
 }
@@ -333,6 +347,8 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn)
     s = skip_space(s);
     done = *s == '\0';
     while (!done) {
+        const char *start = s;
+
         prop = OPENSSL_malloc(sizeof(*prop));
         if (prop == NULL)
             goto err;
@@ -343,11 +359,13 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn)
         prop->oper = PROPERTY_OPER_EQ;
         if (prop->name_idx == 0) {
             PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_PARSE_FAILED);
+            ERR_add_error_data(2, "Unknown name HERE-->", start);
             goto err;
         }
         if (match_ch(&s, '=')) {
             if (!parse_value(ctx, &s, prop, 1)) {
                 PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_NO_VALUE);
+                ERR_add_error_data(2, "HERE-->", start);
                 goto err;
             }
         } else {
@@ -363,6 +381,7 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn)
     }
     if (*s != '\0') {
         PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_TRAILING_CHARACTERS);
+        ERR_add_error_data(2, "HERE-->", s);
         goto err;
     }
     res = stack_to_property_list(sk);
@@ -424,6 +443,7 @@ skip_value:
     }
     if (*s != '\0') {
         PROPerr(PROP_F_OSSL_PARSE_QUERY, PROP_R_TRAILING_CHARACTERS);
+        ERR_add_error_data(2, "HERE-->", s);
         goto err;
     }
     res = stack_to_property_list(sk);