Convert hstore_in to report errors softly.

The error reporting here was not only old and crufty, but untested.
I took the opportunity to bring the messages into some sort of
compliance with our message style guidelines.

Discussion: https://postgr.es/m/6B6A5C77-60AD-4A71-9F3A-B2C026A281A6@dunslane.net

diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out
index 64a3272b9cf4565c886a536ad4e1d6ebbee70aed..d6faa918670bbb6312c76b64e4355d7fd16bc3fa 100644 (file)
--- a/contrib/hstore/expected/hstore.out
+++ b/contrib/hstore/expected/hstore.out
@@ -243,6 +243,40 @@ select '   '::hstore;
  
 (1 row)
 
+-- invalid input
+select '  =>null'::hstore;
+ERROR:  syntax error in hstore, near "=" at position 2
+LINE 1: select '  =>null'::hstore;
+               ^
+select 'aa=>"'::hstore;
+ERROR:  syntax error in hstore: unexpected end of string
+LINE 1: select 'aa=>"'::hstore;
+               ^
+-- also try it with non-error-throwing API
+select pg_input_is_valid('a=>b', 'hstore');
+ pg_input_is_valid 
+-------------------
+ t
+(1 row)
+
+select pg_input_is_valid('a=b', 'hstore');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+select pg_input_error_message('a=b', 'hstore');
+             pg_input_error_message             
+------------------------------------------------
+ syntax error in hstore, near "b" at position 2
+(1 row)
+
+select pg_input_error_message(' =>b', 'hstore');
+             pg_input_error_message             
+------------------------------------------------
+ syntax error in hstore, near "=" at position 1
+(1 row)
+
 -- -> operator
 select 'aa=>b, c=>d , b=>16'::hstore->'c';
  ?column? 

diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index 6161df2790379a75efa48cb577881302869f2030..ae09cede8c0e66cfab7f0e0d2f9d76147bef9994 100644 (file)
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -12,6 +12,7 @@
 #include "hstore.h"
 #include "lib/stringinfo.h"
 #include "libpq/pqformat.h"
+#include "nodes/miscnodes.h"
 #include "utils/builtins.h"
 #include "utils/json.h"
 #include "utils/jsonb.h"
@@ -32,12 +33,17 @@ typedef struct
    char       *cur;
    char       *word;
    int         wordlen;
+   Node       *escontext;
 
    Pairs      *pairs;
    int         pcur;
    int         plen;
 } HSParser;
 
+static bool hstoreCheckKeyLength(size_t len, HSParser *state);
+static bool hstoreCheckValLength(size_t len, HSParser *state);
+
+
 #define RESIZEPRSBUF \
 do { \
        if ( state->cur - state->word + 1 >= state->wordlen ) \
@@ -49,6 +55,32 @@ do { \
        } \
 } while (0)
 
+#define PRSSYNTAXERROR return prssyntaxerror(state)
+
+static bool
+prssyntaxerror(HSParser *state)
+{
+   errsave(state->escontext,
+           (errcode(ERRCODE_SYNTAX_ERROR),
+            errmsg("syntax error in hstore, near \"%.*s\" at position %d",
+                   pg_mblen(state->ptr), state->ptr,
+                   (int) (state->ptr - state->begin))));
+   /* In soft error situation, return false as convenience for caller */
+   return false;
+}
+
+#define PRSEOF return prseof(state)
+
+static bool
+prseof(HSParser *state)
+{
+   errsave(state->escontext,
+           (errcode(ERRCODE_SYNTAX_ERROR),
+            errmsg("syntax error in hstore: unexpected end of string")));
+   /* In soft error situation, return false as convenience for caller */
+   return false;
+}
+
 
 #define GV_WAITVAL 0
 #define GV_INVAL 1
@@ -80,9 +112,7 @@ get_val(HSParser *state, bool ignoreeq, bool *escaped)
            }
            else if (*(state->ptr) == '=' && !ignoreeq)
            {
-               elog(ERROR, "Syntax error near \"%.*s\" at position %d",
-                    pg_mblen(state->ptr), state->ptr,
-                    (int32) (state->ptr - state->begin));
+               PRSSYNTAXERROR;
            }
            else if (*(state->ptr) == '\\')
            {
@@ -139,7 +169,7 @@ get_val(HSParser *state, bool ignoreeq, bool *escaped)
            }
            else if (*(state->ptr) == '\0')
            {
-               elog(ERROR, "Unexpected end of string");
+               PRSEOF;
            }
            else
            {
@@ -151,7 +181,7 @@ get_val(HSParser *state, bool ignoreeq, bool *escaped)
        else if (st == GV_WAITESCIN)
        {
            if (*(state->ptr) == '\0')
-               elog(ERROR, "Unexpected end of string");
+               PRSEOF;
            RESIZEPRSBUF;
            *(state->cur) = *(state->ptr);
            state->cur++;
@@ -160,14 +190,14 @@ get_val(HSParser *state, bool ignoreeq, bool *escaped)
        else if (st == GV_WAITESCESCIN)
        {
            if (*(state->ptr) == '\0')
-               elog(ERROR, "Unexpected end of string");
+               PRSEOF;
            RESIZEPRSBUF;
            *(state->cur) = *(state->ptr);
            state->cur++;
            st = GV_INESCVAL;
        }
        else
-           elog(ERROR, "Unknown state %d at position line %d in file '%s'", st, __LINE__, __FILE__);
+           elog(ERROR, "unrecognized get_val state: %d", st);
 
        state->ptr++;
    }
@@ -180,7 +210,7 @@ get_val(HSParser *state, bool ignoreeq, bool *escaped)
 #define WDEL   4
 
 
-static void
+static bool
 parse_hstore(HSParser *state)
 {
    int         st = WKEY;
@@ -197,14 +227,20 @@ parse_hstore(HSParser *state)
        if (st == WKEY)
        {
            if (!get_val(state, false, &escaped))
-               return;
+           {
+               if (SOFT_ERROR_OCCURRED(state->escontext))
+                   return false;
+               return true;    /* EOF, all okay */
+           }
            if (state->pcur >= state->plen)
            {
                state->plen *= 2;
                state->pairs = (Pairs *) repalloc(state->pairs, sizeof(Pairs) * state->plen);
            }
+           if (!hstoreCheckKeyLength(state->cur - state->word, state))
+               return false;
            state->pairs[state->pcur].key = state->word;
-           state->pairs[state->pcur].keylen = hstoreCheckKeyLen(state->cur - state->word);
+           state->pairs[state->pcur].keylen = state->cur - state->word;
            state->pairs[state->pcur].val = NULL;
            state->word = NULL;
            st = WEQ;
@@ -217,13 +253,11 @@ parse_hstore(HSParser *state)
            }
            else if (*(state->ptr) == '\0')
            {
-               elog(ERROR, "Unexpected end of string");
+               PRSEOF;
            }
            else if (!isspace((unsigned char) *(state->ptr)))
            {
-               elog(ERROR, "Syntax error near \"%.*s\" at position %d",
-                    pg_mblen(state->ptr), state->ptr,
-                    (int32) (state->ptr - state->begin));
+               PRSSYNTAXERROR;
            }
        }
        else if (st == WGT)
@@ -234,27 +268,31 @@ parse_hstore(HSParser *state)
            }
            else if (*(state->ptr) == '\0')
            {
-               elog(ERROR, "Unexpected end of string");
+               PRSEOF;
            }
            else
            {
-               elog(ERROR, "Syntax error near \"%.*s\" at position %d",
-                    pg_mblen(state->ptr), state->ptr,
-                    (int32) (state->ptr - state->begin));
+               PRSSYNTAXERROR;
            }
        }
        else if (st == WVAL)
        {
            if (!get_val(state, true, &escaped))
-               elog(ERROR, "Unexpected end of string");
+           {
+               if (SOFT_ERROR_OCCURRED(state->escontext))
+                   return false;
+               PRSEOF;
+           }
+           if (!hstoreCheckValLength(state->cur - state->word, state))
+               return false;
            state->pairs[state->pcur].val = state->word;
-           state->pairs[state->pcur].vallen = hstoreCheckValLen(state->cur - state->word);
+           state->pairs[state->pcur].vallen = state->cur - state->word;
            state->pairs[state->pcur].isnull = false;
            state->pairs[state->pcur].needfree = true;
            if (state->cur - state->word == 4 && !escaped)
            {
                state->word[4] = '\0';
-               if (0 == pg_strcasecmp(state->word, "null"))
+               if (pg_strcasecmp(state->word, "null") == 0)
                    state->pairs[state->pcur].isnull = true;
            }
            state->word = NULL;
@@ -269,17 +307,15 @@ parse_hstore(HSParser *state)
            }
            else if (*(state->ptr) == '\0')
            {
-               return;
+               return true;
            }
            else if (!isspace((unsigned char) *(state->ptr)))
            {
-               elog(ERROR, "Syntax error near \"%.*s\" at position %d",
-                    pg_mblen(state->ptr), state->ptr,
-                    (int32) (state->ptr - state->begin));
+               PRSSYNTAXERROR;
            }
        }
        else
-           elog(ERROR, "Unknown state %d at line %d in file '%s'", st, __LINE__, __FILE__);
+           elog(ERROR, "unrecognized parse_hstore state: %d", st);
 
        state->ptr++;
    }
@@ -373,6 +409,16 @@ hstoreCheckKeyLen(size_t len)
    return len;
 }
 
+static bool
+hstoreCheckKeyLength(size_t len, HSParser *state)
+{
+   if (len > HSTORE_MAX_KEY_LEN)
+       ereturn(state->escontext, false,
+               (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
+                errmsg("string too long for hstore key")));
+   return true;
+}
+
 size_t
 hstoreCheckValLen(size_t len)
 {
@@ -383,6 +429,16 @@ hstoreCheckValLen(size_t len)
    return len;
 }
 
+static bool
+hstoreCheckValLength(size_t len, HSParser *state)
+{
+   if (len > HSTORE_MAX_VALUE_LEN)
+       ereturn(state->escontext, false,
+               (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
+                errmsg("string too long for hstore value")));
+   return true;
+}
+
 
 HStore *
 hstorePairs(Pairs *pairs, int32 pcount, int32 buflen)
@@ -418,13 +474,17 @@ PG_FUNCTION_INFO_V1(hstore_in);
 Datum
 hstore_in(PG_FUNCTION_ARGS)
 {
+   char       *str = PG_GETARG_CSTRING(0);
+   Node       *escontext = fcinfo->context;
    HSParser    state;
    int32       buflen;
    HStore     *out;
 
-   state.begin = PG_GETARG_CSTRING(0);
+   state.begin = str;
+   state.escontext = escontext;
 
-   parse_hstore(&state);
+   if (!parse_hstore(&state))
+       PG_RETURN_NULL();
 
    state.pcur = hstoreUniquePairs(state.pairs, state.pcur, &buflen);
 

diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql
index a59db66b0aee806d13d36c9c9e7c50482c5b4d98..15f4f71416a6860cf3ec3b491deb215d354a2335 100644 (file)
--- a/contrib/hstore/sql/hstore.sql
+++ b/contrib/hstore/sql/hstore.sql
@@ -53,6 +53,17 @@ select e'\\"a=>q"w'::hstore;
 select ''::hstore;
 select '   '::hstore;
 
+-- invalid input
+select '  =>null'::hstore;
+select 'aa=>"'::hstore;
+
+-- also try it with non-error-throwing API
+select pg_input_is_valid('a=>b', 'hstore');
+select pg_input_is_valid('a=b', 'hstore');
+select pg_input_error_message('a=b', 'hstore');
+select pg_input_error_message(' =>b', 'hstore');
+
+
 -- -> operator
 
 select 'aa=>b, c=>d , b=>16'::hstore->'c';