Thread: byteain bug(?)

I just noticed some unexpected behavior from byteain:

test=# select '\\009'::bytea;?column?
----------\011
(1 row)

test=# select '\\444'::bytea;?column?
----------$
(1 row)

test=# select '\\999'::bytea;?column?
----------\221
(1 row)

The reason is the following code in byteain:
  else if (!isdigit((unsigned char) *tp++) ||     !isdigit((unsigned char) *tp++) ||     !isdigit((unsigned char)
*tp++))  elog(ERROR, "Bad input string for type bytea");
 

It checks for a '\' followed by three digits, but does not attempt to
enforce that the three digits actually produce a valid octal number. Anyone
object to me fixing this?

-- Joe

Should be fixed.  I noticed this myself.  Should we require three digits?


> I just noticed some unexpected behavior from byteain:
> 
> test=# select '\\009'::bytea;
>  ?column?
> ----------
>  \011
> (1 row)
> 
> test=# select '\\444'::bytea;
>  ?column?
> ----------
>  $
> (1 row)
> 
> test=# select '\\999'::bytea;
>  ?column?
> ----------
>  \221
> (1 row)
> 
> The reason is the following code in byteain:
> 
>    else if (!isdigit((unsigned char) *tp++) ||
>       !isdigit((unsigned char) *tp++) ||
>       !isdigit((unsigned char) *tp++))
>     elog(ERROR, "Bad input string for type bytea");
> 
> It checks for a '\' followed by three digits, but does not attempt to
> enforce that the three digits actually produce a valid octal number. Anyone
> object to me fixing this?
> 
> -- Joe
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 

"Joe Conway" <joseph.conway@home.com> writes:
> It checks for a '\' followed by three digits, but does not attempt to
> enforce that the three digits actually produce a valid octal number. Anyone
> object to me fixing this?

Clearly a bug.  Fix away...
        regards, tom lane

>
> Should be fixed.  I noticed this myself.  Should we require three digits?
>
>
> > I just noticed some unexpected behavior from byteain:
> >
> >
> > test=# select '\\009'::bytea;
> >  ?column?
> > ----------
> >  \011
> > (1 row)

<snip>

> >
> > It checks for a '\' followed by three digits, but does not attempt to
> > enforce that the three digits actually produce a valid octal number.
Anyone
> > object to me fixing this?
> >

Based on the thread this morning on patches, I was thinking we should allow
'\\', '\0', or '\###' where ### is any valid octal. At least that's what I
was going to have decode(bytea, 'escape') handle.

-- Joe

> > > It checks for a '\' followed by three digits, but does not attempt to
> > > enforce that the three digits actually produce a valid octal number.
> Anyone
> > > object to me fixing this?
> > >
> 
> Based on the thread this morning on patches, I was thinking we should allow
> '\\', '\0', or '\###' where ### is any valid octal. At least that's what I
> was going to have decode(bytea, 'escape') handle.

Yep, it is way too open right now.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 

> > > > It checks for a '\' followed by three digits, but does not attempt
to
> > > > enforce that the three digits actually produce a valid octal number.
> > Anyone
> > > > object to me fixing this?
> > > >
> >
> > Based on the thread this morning on patches, I was thinking we should
allow
> > '\\', '\0', or '\###' where ### is any valid octal. At least that's what
I
> > was going to have decode(bytea, 'escape') handle.
>
> Yep, it is way too open right now.

On further thought, I think I'll have to not allow '\0' and require '\000'
instead. Otherwise, how should the following be interpreted:

'\0123'

Is that '\0' followed by the literals '1', '2', and '3'? Or is it '\012'
followed by the literal '3'?

So, I'll go with '\\' or '\###' where ### is any valid octal, for both
byteain and decode(bytea, 'escape').

Comments?

-- Joe

> > > > > It checks for a '\' followed by three digits, but does not attempt
> to
> > > > > enforce that the three digits actually produce a valid octal number.
> > > Anyone
> > > > > object to me fixing this?
> > > > >
> > >
> > > Based on the thread this morning on patches, I was thinking we should
> allow
> > > '\\', '\0', or '\###' where ### is any valid octal. At least that's what
> I
> > > was going to have decode(bytea, 'escape') handle.
> >
> > Yep, it is way too open right now.
> 
> On further thought, I think I'll have to not allow '\0' and require '\000'
> instead. Otherwise, how should the following be interpreted:
> 
> '\0123'
> 
> Is that '\0' followed by the literals '1', '2', and '3'? Or is it '\012'
> followed by the literal '3'?
> 
> So, I'll go with '\\' or '\###' where ### is any valid octal, for both
> byteain and decode(bytea, 'escape').

Very good point.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026