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

[Sutou Kouhei]

Hi,

In <d838025aceeb19c9ff1db702fa55c...@postgrespro.ru>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on 
Mon, 03 Feb 2025 13:38:04 +0700,
  Vladlen Popolitov <v.popoli...@postgrespro.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