Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/postgrest/src/postgrest/_async/request_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from ..utils import model_validate_json

ReqConfig = RequestConfig[AsyncClient]
QueryBuilderT = TypeVar("QueryBuilderT", bound="AsyncQueryRequestBuilder")


def get_retry_delay(resp: Response, attempt_count: int) -> int:
Expand Down Expand Up @@ -59,6 +60,17 @@ class AsyncQueryRequestBuilder:
def __init__(self, request: ReqConfig):
self.request = request

def select(self: QueryBuilderT, *columns: str) -> QueryBuilderT:
_, params, _, _ = pre_select(*columns, count=None)
self.request.params = self.request.params.add("select", params["select"])
if prefer_headers := self.request.headers.get_list("Prefer", split_commas=True):
prefer_headers = [h for h in prefer_headers if not h.startswith("return=")]
prefer_headers.append("return=representation")
self.request.headers["Prefer"] = ",".join(prefer_headers)
else:
self.request.headers["Prefer"] = "return=representation"
return self

def retry(self, enabled: bool) -> Self:
self.request.retry_enabled = enabled
return self
Comment on lines +63 to +72
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

PostgREST Prefer header precedence return=minimal return=representation multiple directives

💡 Result:

In PostgREST, Prefer follows RFC 7240. PostgREST supports Prefer: return=minimal (default for writes) and Prefer: return=representation (return the mutated rows). [1][2]

If you send multiple return directives in the same request, e.g.

  • Prefer: return=minimal, return=representation (or across multiple Prefer: header lines)

then only the first occurrence is considered and later duplicates should be ignored, per RFC 7240 (“If any preference is specified more than once, only the first instance is to be considered.”). So the leftmost return=... wins. [2]

Also, RFC 7240 notes return=minimal and return=representation are mutually exclusive, so including both is generally a client bug. [2]

Sources: [1] PostgREST Prefer header docs, [2] RFC 7240.


🌐 Web query:

PostgREST RFC 7240 Prefer header handling duplicate directives

💡 Result:

RFC 7240 defines two relevant rules for Prefer:

  • Multiple Prefer header fields are allowed and are equivalent to a single header whose value is the comma-separated concatenation of all preference tokens. If an HTTP client/library “merges” multiple Prefer headers into one comma-separated header, that’s still compliant. [1]
  • Duplicate preference tokens should not be sent, but if they are, only the first occurrence is considered; later duplicates should be ignored without error (ordering becomes significant only in this duplicate case). [1]

PostgREST follows this RFC 7240 behavior when parsing request headers: if a preference is set more than once (even across multiple Prefer headers), PostgREST uses the first value and ignores subsequent duplicates. Example from PostgREST’s own fromHeaders docs: Prefer: tx=commit, tx=rollback results in tx=commit being used. [2]

Separately, PostgREST’s Prefer: handling=strict is about invalid/unrecognized/unfulfillable preferences (error vs ignore), not about duplicates—duplicates are simply handled as “first wins” per RFC 7240. [3]

Sources: RFC 7240 Prefer header specification [1]; PostgREST preference parsing (fromHeaders) behavior [2]; PostgREST Prefer header docs / handling=strict [3].

[1] https://www.rfc-editor.org/rfc/rfc7240
[2] https://hackage.haskell.org/package/postgrest/docs/PostgREST-Request-Preferences.html
[3] https://docs.postgrest.org/en/v12/references/api/preferences.html


🏁 Script executed:

# Search for SyncQueryRequestBuilder.select to verify if logic is mirrored
fd -type f -name "*.py" | xargs rg -l "SyncQueryRequestBuilder" | head -5

Repository: supabase/supabase-py

Length of output: 444


🏁 Script executed:

# Find and examine the SyncQueryRequestBuilder.select method
rg -A 15 "class SyncQueryRequestBuilder" --type py

Repository: supabase/supabase-py

Length of output: 1614


Replace return= directives in Prefer header instead of appending to avoid return=minimal overriding the intended behavior.

PostgREST follows RFC 7240, which specifies that when a preference appears multiple times, only the first occurrence is honored. If Prefer already contains return=minimal and select() appends ,return=representation, the header becomes return=minimal,return=representation—and PostgREST will use return=minimal (the first), not the intended return=representation. This breaks embedding behavior.

Replace any existing return= preference instead of appending to ensure return=representation is always used:

Required fix
-        prefer_header = self.request.headers.get("Prefer")
-        if not prefer_header:
-            self.request.headers["Prefer"] = "return=representation"
-        elif "return=representation" not in [
-            value.strip() for value in prefer_header.split(",")
-        ]:
-            self.request.headers["Prefer"] = f"{prefer_header},return=representation"
+        prefer_header = self.request.headers.get("Prefer")
+        if not prefer_header:
+            self.request.headers["Prefer"] = "return=representation"
+        else:
+            values = [v.strip() for v in prefer_header.split(",") if v.strip()]
+            values = [v for v in values if not v.lower().startswith("return=")]
+            values.append("return=representation")
+            self.request.headers["Prefer"] = ",".join(values)

Apply the same fix to SyncQueryRequestBuilder.select in src/postgrest/src/postgrest/_sync/request_builder.py to keep them synchronized.

🤖 Prompt for AI Agents
In `@src/postgrest/src/postgrest/_async/request_builder.py` around lines 36 - 46,
In select (the async QueryBuilder.select implementation) the code currently
appends "return=representation" to the Prefer header which can leave an existing
"return=minimal" first and be ignored by PostgREST; change the logic to parse
self.request.headers.get("Prefer") and if any existing "return=" directive
exists replace that fragment with "return=representation" (preserving other
comma-separated preferences and whitespace), otherwise add
"return=representation" as before; apply the same change to the
SyncQueryRequestBuilder.select implementation so both _async/request_builder.py
and _sync/request_builder.py consistently replace any existing "return="
preference instead of appending.

Expand Down
12 changes: 12 additions & 0 deletions src/postgrest/src/postgrest/_sync/request_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from ..utils import model_validate_json

ReqConfig = RequestConfig[Client]
QueryBuilderT = TypeVar("QueryBuilderT", bound="SyncQueryRequestBuilder")


def get_retry_delay(resp: Response, attempt_count: int) -> int:
Expand Down Expand Up @@ -59,6 +60,17 @@ class SyncQueryRequestBuilder:
def __init__(self, request: ReqConfig):
self.request = request

def select(self: QueryBuilderT, *columns: str) -> QueryBuilderT:
_, params, _, _ = pre_select(*columns, count=None)
self.request.params = self.request.params.add("select", params["select"])
if prefer_headers := self.request.headers.get_list("Prefer", split_commas=True):
prefer_headers = [h for h in prefer_headers if not h.startswith("return=")]
prefer_headers.append("return=representation")
self.request.headers["Prefer"] = ",".join(prefer_headers)
else:
self.request.headers["Prefer"] = "return=representation"
return self

def retry(self, enabled: bool) -> Self:
self.request.retry_enabled = enabled
return self
Expand Down
34 changes: 33 additions & 1 deletion src/postgrest/tests/_async/test_request_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from postgrest import AsyncRequestBuilder, AsyncSingleRequestBuilder
from postgrest._async.request_builder import RequestConfig
from postgrest.base_request_builder import APIResponse, SingleAPIResponse
from postgrest.types import JSON, CountMethod
from postgrest.types import JSON, CountMethod, ReturnMethod


@pytest.fixture
Expand Down Expand Up @@ -120,6 +120,26 @@ def test_upsert(self, request_builder: AsyncRequestBuilder):
assert builder.request.http_method == "POST"
assert builder.request.json == {"key1": "val1"}

def test_insert_with_select(self, request_builder: AsyncRequestBuilder):
builder = request_builder.insert({"key1": "val1"}).select("id", "key1")

assert builder.request.params["select"] == "id,key1"
assert builder.request.headers.get_list("prefer", True) == [
"return=representation"
]

def test_insert_with_select_forces_representation(
self, request_builder: AsyncRequestBuilder
):
builder = request_builder.insert(
{"key1": "val1"}, returning=ReturnMethod.minimal
).select("id")

assert builder.request.params["select"] == "id"
assert builder.request.headers.get_list("prefer", True) == [
"return=representation",
]

def test_bulk_upsert_with_default(self, request_builder: AsyncRequestBuilder):
builder = request_builder.upsert(
[{"key1": "val1", "key2": "val2"}, {"key3": "val3"}], default_to_null=False
Expand Down Expand Up @@ -168,6 +188,12 @@ def test_update_with_max_affected(self, request_builder: AsyncRequestBuilder):
assert builder.request.http_method == "PATCH"
assert builder.request.json == {"key1": "val1"}

def test_update_with_select(self, request_builder: AsyncRequestBuilder):
builder = request_builder.update({"key1": "val1"}).eq("id", 1).select("id")

assert builder.request.params["id"] == "eq.1"
assert builder.request.params["select"] == "id"


class TestDelete:
def test_delete(self, request_builder: AsyncRequestBuilder):
Expand Down Expand Up @@ -198,6 +224,12 @@ def test_delete_with_max_affected(self, request_builder: AsyncRequestBuilder):
assert builder.request.http_method == "DELETE"
assert builder.request.json == {}

def test_delete_with_select(self, request_builder: AsyncRequestBuilder):
builder = request_builder.delete().eq("id", 1).select("id")

assert builder.request.params["id"] == "eq.1"
assert builder.request.params["select"] == "id"


class TestTextSearch:
def test_text_search(self, request_builder: AsyncRequestBuilder):
Expand Down
34 changes: 33 additions & 1 deletion src/postgrest/tests/_sync/test_request_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from postgrest import SyncRequestBuilder, SyncSingleRequestBuilder
from postgrest._async.request_builder import RequestConfig
from postgrest.base_request_builder import APIResponse, SingleAPIResponse
from postgrest.types import JSON, CountMethod
from postgrest.types import JSON, CountMethod, ReturnMethod


@pytest.fixture
Expand Down Expand Up @@ -120,6 +120,26 @@ def test_upsert(self, request_builder: SyncRequestBuilder):
assert builder.request.http_method == "POST"
assert builder.request.json == {"key1": "val1"}

def test_insert_with_select(self, request_builder: SyncRequestBuilder):
builder = request_builder.insert({"key1": "val1"}).select("id", "key1")

assert builder.request.params["select"] == "id,key1"
assert builder.request.headers.get_list("prefer", True) == [
"return=representation"
]

def test_insert_with_select_forces_representation(
self, request_builder: SyncRequestBuilder
):
builder = request_builder.insert(
{"key1": "val1"}, returning=ReturnMethod.minimal
).select("id")

assert builder.request.params["select"] == "id"
assert builder.request.headers.get_list("prefer", True) == [
"return=representation",
]

def test_bulk_upsert_with_default(self, request_builder: SyncRequestBuilder):
builder = request_builder.upsert(
[{"key1": "val1", "key2": "val2"}, {"key3": "val3"}], default_to_null=False
Expand Down Expand Up @@ -168,6 +188,12 @@ def test_update_with_max_affected(self, request_builder: SyncRequestBuilder):
assert builder.request.http_method == "PATCH"
assert builder.request.json == {"key1": "val1"}

def test_update_with_select(self, request_builder: SyncRequestBuilder):
builder = request_builder.update({"key1": "val1"}).eq("id", 1).select("id")

assert builder.request.params["id"] == "eq.1"
assert builder.request.params["select"] == "id"


class TestDelete:
def test_delete(self, request_builder: SyncRequestBuilder):
Expand Down Expand Up @@ -198,6 +224,12 @@ def test_delete_with_max_affected(self, request_builder: SyncRequestBuilder):
assert builder.request.http_method == "DELETE"
assert builder.request.json == {}

def test_delete_with_select(self, request_builder: SyncRequestBuilder):
builder = request_builder.delete().eq("id", 1).select("id")

assert builder.request.params["id"] == "eq.1"
assert builder.request.params["select"] == "id"


class TestTextSearch:
def test_text_search(self, request_builder: SyncRequestBuilder):
Expand Down