kaxil opened a new pull request, #71018:
URL: https://github.com/apache/airflow/pull/71018

   Fixes #71010
   Closes https://github.com/apache/airflow/pull/71015
   
   ## Problem
   
   `VariableBody.value` is typed `JsonValue`, so the Variables REST API accepts 
any JSON type, but everything downstream assumed a string:
   
   - `POST /api/v2/variables` with `"value": ["a", "b"]` returned 201 but 
stored the Python repr `['a', 'b']`. That is not valid JSON, so the variable 
silently breaks the next `Variable.get(key, deserialize_json=True)`.
   - `PATCH /api/v2/variables/{key}` with the same payload failed with a masked 
500: the raw list reached the Fernet encryption step, which raised `TypeError: 
encoding without a string argument`.
   - Bulk update failed the same way, and one non-string entity took down the 
whole request. Bulk create already JSON-encoded dicts and lists, but stored 
booleans and nulls as `"True"`/`"None"`.
   
   ## Solution
   
   `VariableBody` now JSON-encodes non-string values once, at request 
validation, so POST, PATCH and both bulk actions behave identically: strings 
are stored verbatim, everything else is stored as JSON (`indent=2`, 
byte-identical to the existing bulk-create format) and round-trips through 
`deserialize_json=True`. The now-redundant `serialize_json` special case in 
bulk create is removed, and `Variable.set_val` raises a clear `TypeError` 
pointing at `serialize_json=True` instead of the cryptic encoding error.
   
   | Request | Before | After |
   |---|---|---|
   | POST `"value": ["a", "b"]` | 201, stores `['a', 'b']`, unreadable as JSON 
| 201, stores `["a", "b"]` |
   | PATCH `"value": ["a", "b"]` | 500 | 200 |
   | Bulk update with a dict value | Whole request 500 | 200 |
   | POST or bulk `"value": true` / `null` | Stores `"True"` / `"None"` | 
Stores `true` / `null` |
   | PATCH `"value": null` | 200 but silently kept the old value | 200, stores 
`null` |
   
   String values, including JSON passed as a string, are stored byte-for-byte 
as before, and the OpenAPI schema is unchanged, so generated clients are 
unaffected.
   
   ## Why coerce instead of rejecting with 422
   
   Non-string values are an intentional part of the API contract since #49844: 
the UI Import Variables flow sends raw parsed JSON 
([ImportVariablesForm.tsx#L71](https://github.com/apache/airflow/blob/b316afb44d/airflow-core/src/airflow/ui/src/pages/Variables/ImportVariablesForm.tsx#L71)),
 the docs describe bulk-uploading variables as a JSON file, and `airflow 
variables import` applies the identical encode-iff-not-a-string rule 
([variable_command.py#L165](https://github.com/apache/airflow/blob/b316afb44d/airflow-core/src/airflow/cli/commands/variable_command.py#L165)).
 Tightening `value` back to `str` would 422 all of those.
   
   ## Notes
   
   - Rows already corrupted on released versions stay as-is; a stored Python 
repr cannot be safely auto-converted back to JSON.
   - Non-finite floats still store as the non-strict-JSON literal `NaN`. 
Rejecting them in the validator is not viable: the 422 response echoes the NaN 
input, which starlette's `JSONResponse` cannot serialize, turning the rejection 
into a 500.
   - The middleware issue from the report (`JWTRefreshMiddleware` masking the 
real traceback as `RuntimeError: No response returned`) is not addressed here; 
it also affects #68868 and #66889.
   


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