Re: hstore parser incorrectly handles malformed input - Mailing list pgsql-bugs

From Tom Lane
Subject Re: hstore parser incorrectly handles malformed input
Date
Msg-id 19224.1335489137@sss.pgh.pa.us
Whole thread Raw
In response to Re: hstore parser incorrectly handles malformed input  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: hstore parser incorrectly handles malformed input  (Vik Reykja <vikreykja@gmail.com>)
Re: hstore parser incorrectly handles malformed input  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-bugs
I wrote:
> Ryan Kelly <rpkelly22@gmail.com> writes:
>> In my mind, all of these should have been rejected as erroneous input.
>> To that end, I have attached a patch which causes all of these inputs
>> to be rejected as invalid.

> Hm, I would have expected all three of these to result in "a" having
> an empty-string value.  I see nothing in the hstore documentation
> suggesting that I must write a=>"" or some such to get an empty value,

Attached is an alternative patch that fixes it that way.

Does anybody else have an opinion as to which of these solutions is
more preferable?  And should we regard this as a back-patchable bug
fix, or a definition change suitable only for HEAD?

            regards, tom lane

diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index dde6c4b376ea2a82ed7d3cc0ef040e52f4df393a..08eafa18f1600d21e35eb4c3298215318dfebd9d 100644
*** a/contrib/hstore/hstore_io.c
--- b/contrib/hstore/hstore_io.c
*************** get_val(HSParser *state, bool ignoreeq,
*** 74,81 ****
--- 74,88 ----
              }
              else if (*(state->ptr) == '=' && !ignoreeq)
              {
+                 /* Empty key is a syntax error */
                  elog(ERROR, "Syntax error near '%c' at position %d", *(state->ptr), (int4) (state->ptr -
state->begin));
              }
+             else if (*(state->ptr) == ',' && ignoreeq)
+             {
+                 /* Empty value is perfectly OK */
+                 state->ptr--;
+                 return true;
+             }
              else if (*(state->ptr) == '\\')
              {
                  st = GV_WAITESCIN;
*************** parse_hstore(HSParser *state)
*** 191,197 ****
          if (st == WKEY)
          {
              if (!get_val(state, false, &escaped))
!                 return;
              if (state->pcur >= state->plen)
              {
                  state->plen *= 2;
--- 198,204 ----
          if (st == WKEY)
          {
              if (!get_val(state, false, &escaped))
!                 return;            /* end of string */
              if (state->pcur >= state->plen)
              {
                  state->plen *= 2;
*************** parse_hstore(HSParser *state)
*** 236,242 ****
          else if (st == WVAL)
          {
              if (!get_val(state, true, &escaped))
!                 elog(ERROR, "Unexpected end of string");
              state->pairs[state->pcur].val = state->word;
              state->pairs[state->pcur].vallen = hstoreCheckValLen(state->cur - state->word);
              state->pairs[state->pcur].isnull = false;
--- 243,252 ----
          else if (st == WVAL)
          {
              if (!get_val(state, true, &escaped))
!             {
!                 /* end of string, treat as empty value */
!                 state->ptr--;
!             }
              state->pairs[state->pcur].val = state->word;
              state->pairs[state->pcur].vallen = hstoreCheckValLen(state->cur - state->word);
              state->pairs[state->pcur].isnull = false;

pgsql-bugs by date:

Previous
From: Josh Berkus
Date:
Subject: Re: log_collector doesn't respond to reloads
Next
From: Tom Lane
Date:
Subject: Re: log_collector doesn't respond to reloads