Re: [HACKERS] Timeline ID in backup_label file

From: David Steele <david(at)pgmasters(dot)net>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Subject: Re: [HACKERS] Timeline ID in backup_label file
Date: 2017-11-27 14:06:23
Message-ID: 67417866-751e-a138-3dcf-6e1d32981472@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Michael,

On 11/15/17 10:09 PM, Michael Paquier wrote:
> On Thu, Nov 16, 2017 at 9:20 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>> For this patch at least, I think we should do #1. Getting rid of the order
>> dependency is attractive but there may be other programs that are depending
>> on the order. I know you are not proposing to change the order now, but it
>> *could* be changed with #2.
>
> read_backup_label() is a static function in the backend code. With #2
> I do not imply to change the order of the elements written in the
> backup_label file, just to make the way they are parsed smarter.

My point is that the order *could* be changed and it wouldn't be noticed
if the read function were improved as you propose.

I'm not against the idea, just think we should continue to enforce the
order unless we decide an interface break is OK.

>> Also, I think DEBUG1 would be a more appropriate log level if any logging is
>> done.
>
> OK, the label and time strings can include spaces. The label string is
> assumed to not be bigger than 1028 bytes, and the timestamp is assumed
> that it can get up to 128. So it is possible to use stuff like
> fscanf("%127[^\n]\n") to avoid any buffer overflow problems on the
> backend. If a backup_label is generated with strings longer than that,
> then read_backup_label would fail to parse the next entries but that's
> not really something to worry about IMO as those are only minor sanity
> checks. Having a safer coding in the backend is more important, and
> the new checks should not trigger any hard failures.

I have tested and get an error as expected when I munge the backup_label
file:

FATAL: invalid data in file "backup_label"
DETAIL: Timeline ID parsed is 2, but expected 1

Everything else looks good so I will mark it ready for committer.

Regards,
--
-David
david(at)pgmasters(dot)net

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Oliver Ford 2017-11-27 15:01:06 Re: Add RANGE with values and exclusions clauses to the Window Functions
Previous Message Justin Pryzby 2017-11-27 13:33:38 Re: PG10.1 autovac killed building extended stats