Date: Tue, 04 Feb 2025 15:29:27 +0900 (JST)
Message-Id: <20250204.152927.1922290693174479861.kou@clear-code.com>
To: v.popolitov@postgrespro.ru
Cc: sawada.mshk@gmail.com, zhjwpku@gmail.com, michael@paquier.xyz,
 pgsql-hackers@postgresql.org
Subject: Re: Make COPY format extendable: Extract COPY TO format
 implementations
From: Sutou Kouhei <kou@clear-code.com>
In-Reply-To: <d838025aceeb19c9ff1db702fa55cabf@postgrespro.ru>
References: <CAD21AoA3KMddnjxY1hxth3f4f1wo8a8i2icgK6GEKqXNR_e6jA@mail.gmail.com>
	<20250201.191201.966818981739331450.kou@clear-code.com>
	<d838025aceeb19c9ff1db702fa55cabf@postgrespro.ru>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20250204.152927.1922290693174479861.kou%40clear-code.com>
Precedence: bulk

Hi,

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