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
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 |