Remove unnecessary code rom be_lo_put()
Hi,
I noticed that a permission check is performed in be_lo_put()
just after returning inv_open(), but teh permission should be
already checked in inv_open(), so I think we can remove this
part of codes. I attached a patch for this fix.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>
Attachments:
remove_unnecessary_code_from_be_lo_put.patchtext/x-diff; name=remove_unnecessary_code_from_be_lo_put.patchDownload
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 28ad1c9277..f728d0346c 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -860,17 +860,6 @@ be_lo_put(PG_FUNCTION_ARGS)
lo_cleanup_needed = true;
loDesc = inv_open(loOid, INV_WRITE, CurrentMemoryContext);
- /* Permission check */
- if (!lo_compat_privileges &&
- pg_largeobject_aclcheck_snapshot(loDesc->id,
- GetUserId(),
- ACL_UPDATE,
- loDesc->snapshot) != ACLCHECK_OK)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied for large object %u",
- loDesc->id)));
-
inv_seek(loDesc, offset, SEEK_SET);
written = inv_write(loDesc, VARDATA_ANY(str), VARSIZE_ANY_EXHDR(str));
Assert(written == VARSIZE_ANY_EXHDR(str));
On 24.04.24 11:59, Yugo NAGATA wrote:
I noticed that a permission check is performed in be_lo_put()
just after returning inv_open(), but teh permission should be
already checked in inv_open(), so I think we can remove this
part of codes. I attached a patch for this fix.
Yes, I think you are right.
This check was added in 8d9881911f0, but then the refactoring in
ae20b23a9e7 should probably have removed it.
Peter Eisentraut <peter@eisentraut.org> writes:
On 24.04.24 11:59, Yugo NAGATA wrote:
I noticed that a permission check is performed in be_lo_put()
just after returning inv_open(), but teh permission should be
already checked in inv_open(), so I think we can remove this
part of codes. I attached a patch for this fix.
Yes, I think you are right.
I agree. Do you want to do the honors?
regards, tom lane
On Wed, Apr 24, 2024 at 09:25:09AM -0400, Tom Lane wrote:
I agree. Do you want to do the honors?
Good catch. The same check happens when the object is opened. Note
that you should be able to remove utils/acl.h at the top of
be-fsstubs.c as this would remove the last piece of code that does an
ACL check in this file. No objections with doing that now, removing
this code.
--
Michael
On 25.04.24 01:50, Michael Paquier wrote:
On Wed, Apr 24, 2024 at 09:25:09AM -0400, Tom Lane wrote:
I agree. Do you want to do the honors?
Good catch. The same check happens when the object is opened. Note
that you should be able to remove utils/acl.h at the top of
be-fsstubs.c as this would remove the last piece of code that does an
ACL check in this file. No objections with doing that now, removing
this code.
utils/acl.h is still needed for object_ownercheck() called in
be_lo_unlink().
On 24.04.24 15:25, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
On 24.04.24 11:59, Yugo NAGATA wrote:
I noticed that a permission check is performed in be_lo_put()
just after returning inv_open(), but teh permission should be
already checked in inv_open(), so I think we can remove this
part of codes. I attached a patch for this fix.Yes, I think you are right.
I agree. Do you want to do the honors?
done
On Thu, 25 Apr 2024 10:26:41 +0200
Peter Eisentraut <peter@eisentraut.org> wrote:
On 24.04.24 15:25, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
On 24.04.24 11:59, Yugo NAGATA wrote:
I noticed that a permission check is performed in be_lo_put()
just after returning inv_open(), but teh permission should be
already checked in inv_open(), so I think we can remove this
part of codes. I attached a patch for this fix.Yes, I think you are right.
I agree. Do you want to do the honors?
done
Thank you!
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>