Re: Make COPY format extendable: Extract COPY TO format implementations

From: Sutou Kouhei <kou(at)clear-code(dot)com>
To: v(dot)popolitov(at)postgrespro(dot)ru
Cc: sawada(dot)mshk(at)gmail(dot)com, zhjwpku(at)gmail(dot)com, michael(at)paquier(dot)xyz, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: 2025-02-04 06:29:27
Message-ID: 20250204.152927.1922290693174479861.kou@clear-code.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

In <d838025aceeb19c9ff1db702fa55cabf(at)postgrespro(dot)ru>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 03 Feb 2025 13:38:04 +0700,
Vladlen Popolitov <v(dot)popolitov(at)postgrespro(dot)ru> wrote:

> I would like to inform about the security breach in your design of
> COPY TO/FROM.

Thanks! I didn't notice it.

> You use FORMAT option to add new formats, filling it with routine name
> in shared library. As result any caller can call any routine in
> PostgreSQL kernel.

We require "FORMAT_NAME(internal)" signature:

----
funcargtypes[0] = INTERNALOID;
handlerOid = LookupFuncName(list_make1(makeString(format)), 1,
funcargtypes, true);
----

So any caller can call only routines that use the signature.

Should we add more checks for security? If so, what checks
are needed?

For example, does requiring a prefix such as "copy_" (use
"copy_json" for "json" format) improve security?

For example, we need to register a handler explicitly
(CREATE ACCESS METHOD) when we want to use a new access
method. Should we require an explicit registration for
custom COPY format too?

> Standard PostgreSQL realisation for new methods to use USING
> keyword. Every
> new method could have own options (FORMAT is option of internal 'copy
> from/to'
> methods),

Ah, I didn't think about USING.

You suggest "COPY ... USING json" not "COPY ... FORMAT json"
like "CREATE INDEX ... USING custom_index", right? It will
work. If we use this interface, we should reject "COPY
... FORMAT ... USING" (both of FORMAT/USING are specified).

> it assumes some SetOptions interface, that defines
> an options structure according to the new method requirements.

Sorry. I couldn't find the SetOptions interface in source
code. I found only AT_SetOptions. Did you mean it by "some
SetOptions interface"?

I'm familiar with only access method. It has
IndexAmRoutine::amoptions. Is it a SetOptions interface
example?

FYI: The current patch set doesn't have custom options
support yet. Because we want to start from a minimal feature
set. But we'll add support for custom options eventually.

Thanks,
--
kou

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2025-02-04 06:31:13 Re: Increased work_mem for "logical replication tablesync worker" only?
Previous Message Nisha Moond 2025-02-04 06:22:28 Re: Avoid updating inactive_since for invalid replication slots