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

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? 

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",
+				PRSSYNTAXERROR;
-					 pg_mblen(state->ptr), state->ptr,
-					 (int32) (state->ptr - state->begin));
 			}
 			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",
+				PRSSYNTAXERROR;
-					 pg_mblen(state->ptr), state->ptr,
-					 (int32) (state->ptr - state->begin));
 			}
 		}
 		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",
+				PRSSYNTAXERROR;
-					 pg_mblen(state->ptr), state->ptr,
-					 (int32) (state->ptr - state->begin));
 			}
 		}
 		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",
+				PRSSYNTAXERROR;
-					 pg_mblen(state->ptr), state->ptr,
-					 (int32) (state->ptr - state->begin));
 			}
 		}
 		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);
 

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';