Horton

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

Sutou Kouhei <kou@clear-code.com>

View thread View on postgr.es
From: Sutou Kouhei <kou@clear-code.com>
To: v.popolitov@postgrespro.ru
Cc: sawada.mshk@gmail.com, zhjwpku@gmail.com, michael@paquier.xyz, pgsql-hackers@postgresql.org
Date: 2025-02-04T06:29:27Z
Lists: pgsql-hackers 
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

Commits

  1. Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

  2. Refactor COPY FROM to use format callback functions.

  3. Refactor COPY TO to use format callback functions.

  4. Another try to fix BF failure introduced in commit ddd5f4f54a.

  5. Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

  6. Improve COPY TO performance when server and client encodings match

  7. Simplify signature of CopyAttributeOutCSV() in copyto.c

  8. Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

  9. Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

  10. Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

  11. Add progress reporting of skipped tuples during COPY FROM.

  12. pgbench: Add \syncpipeline

  13. meson: Make gzip and tar optional

  14. Export the external file reader used in COPY FROM as APIs.