Andrushika commented on code in PR #71003:
URL: https://github.com/apache/airflow/pull/71003#discussion_r3961127773


##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -2056,3 +2056,56 @@ def _optional_boolean(value: bool | None) -> bool | None:
         )
     ),
 ]
+
+
+# Update mask
+def validate_update_mask(patch_body_type: type[BaseModel], update_mask: 
list[str] | None) -> list[str] | None:
+    """
+    Reject ``update_mask`` entries that name no field of ``patch_body_type``.
+
+    Every caller narrows the patch down with ``set(update_mask)``, which drops 
an entry matching
+    nothing -- so a typo used to make the whole request a no-op that still 
answered ``200``.
+    Aliases count as known names because that is what a caller sends in the 
body and reads back in
+    the response; which of the two a given endpoint acts on is left untouched 
here. Surrounding
+    whitespace is stripped so a stray space selects the field instead of 
silently selecting nothing.
+
+    Routes take the mask from the query string through 
:func:`update_mask_param_factory`; this is
+    for the bulk endpoints, which carry it in the request body instead.
+
+    :param patch_body_type: the request body type the mask selects fields from.
+    :param update_mask: the requested field names, or ``None``.
+    :return: the mask with whitespace stripped, or ``None``.
+    :raises HTTPException: 400 if any entry names no field.
+    """
+    if not update_mask:
+        return update_mask
+
+    fields = patch_body_type.model_fields
+    known = set(fields) | {
+        alias
+        for field in fields.values()
+        for alias in (field.alias, field.serialization_alias, 
field.validation_alias)

Review Comment:
   I think we should exclude `serialization_alias` here, it is too wide. It is 
the output name, not a name the body can send.
   
   For example: `PATCH /variables/k?update_mask=val` passes this check 
(`VariableBody.value` has `serialization_alias="val"`), but `model_fields_set` 
only has `value`, so it is a 200 no-op again.
   
   validation_alias alone should be enough, since alias fills it too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to