Re: [HACKERS] logical decoding of two-phase transactions

On Mon, Jul 19, 2021 at 4:41 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Jul 19, 2021 at 9:19 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 19, 2021 at 12:43 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Jul 19, 2021 at 1:55 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > >
> > > > Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> > > > > Pushed.
> > > >
> > > > I think you'd be way better off making the gid fields be "char *"
> > > > and pstrdup'ing the result of pq_getmsgstring. Another possibility
> > > > perhaps is to use strlcpy, but I'd only go that way if it's important
> > > > to constrain the received strings to 200 bytes.
> > > >
> > >
> > > I think it is important to constrain length to 200 bytes for this case
> > > as here we receive a prepared transaction identifier which according
> > > to docs [1] has a max length of 200 bytes. Also, in
> > > ParseCommitRecord() and ParseAbortRecord(), we are using strlcpy with
> > > 200 as max length to copy prepare transaction identifier. So, I think
> > > it is better to use strlcpy here unless you or Peter feels otherwise.
> > >
> >
> > OK. I have implemented this reported [1] potential buffer overrun
> > using the constraining strlcpy, because the GID limitation of 200
> > bytes is already mentioned in the documentation [2].
> >
>
> This will work but I think it is better to use sizeof gid buffer as we
> are using in ParseCommitRecord() and ParseAbortRecord(). Tomorrow, if
> due to some unforeseen reason if we change the size of gid buffer to
> be different than the GIDSIZE then it will work seamlessly.
>

Modified as requested. PSA patch v2.

------
Kind Regards,
Peter Smith.
Fujitsu Australia