Re: backup manifests

Hi,

On 2020-03-31 22:15:04 -0700, Noah Misch wrote:
> On Tue, Mar 31, 2020 at 03:50:34PM -0700, Andres Freund wrote:
> > On 2020-03-31 14:10:34 -0400, Robert Haas wrote:
> > > +/*
> > > + * Attempt to parse the WAL files required to restore from backup using
> > > + * pg_waldump.
> > > + */
> > > +static void
> > > +parse_required_wal(validator_context *context, char *pg_waldump_path,
> > > +                   char *wal_directory, manifest_wal_range *first_wal_range)
> > > +{
> > > +    manifest_wal_range *this_wal_range = first_wal_range;
> > > +
> > > +    while (this_wal_range != NULL)
> > > +    {
> > > +        char *pg_waldump_cmd;
> > > +
> > > +        pg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%u --start=%X/%X --end=%X/%X\n",
> > > +               pg_waldump_path, wal_directory, this_wal_range->tli,
> > > +               (uint32) (this_wal_range->start_lsn >> 32),
> > > +               (uint32) this_wal_range->start_lsn,
> > > +               (uint32) (this_wal_range->end_lsn >> 32),
> > > +               (uint32) this_wal_range->end_lsn);
> > > +        if (system(pg_waldump_cmd) != 0)
> > > +            report_backup_error(context,
> > > +                                "WAL parsing failed for timeline %u",
> > > +                                this_wal_range->tli);
> > > +
> > > +        this_wal_range = this_wal_range->next;
> > > +    }
> > > +}
> > 
> > Should we have a function to properly escape paths in cases like this?
> > Not that it's likely or really problematic, but the quoting for path
> > could be "circumvented".
> 
> Are you looking for appendShellString(), or something different?

Looks like that'd be it. Thanks.

Greetings,

Andres Freund