Fix: Validate and handle invalid extra field in connections UI and API (#53963)

[Prasanth345]

This PR addresses issue #53963 by ensuring the extra field in connections is always a valid JSON object, both in the UI and backend.

Summary of Changes

• Frontend (ConnectionForm):

  • Added validation to prevent submission of extra values that are not JSON objects (e.g., "abc", [], 123).
  • Displays a helpful error message to the user if invalid input is detected.

• Backend (ConnectionBody schema):

  • Added a Pydantic validator to enforce that extra is a JSON-encoded Python dict.
  • If extra is invalid or not a dict, the API now returns a 422 Unprocessable Entity with a descriptive error message instead of crashing with a 500.

• Backend (_mask_connection_fields):

  • Updated masking logic to safely handle non-dict JSON or malformed input in extra field.
  • Prevents .items() call on string/array and returns a fallback message instead.

Related Issue

Closes #53963

[shubhamraj-git]

Thanks for the PR.

Airflow 2.x supported "" in extra parameter for connection and is still supported from the airflow cli. Since upcoming airflowctl would use APIs, I think problem would come if someone tries to import connections from older version of airflow, breaking compatibility. We might need to handle that.

cc: @bugraoz93 @pierrejeambrun

[bugraoz93]

Thanks for the PR.

Airflow 2.x supported "" in extra parameter for connection and is still supported from the airflow cli. Since upcoming airflowctl would use APIs, I think problem would come if someone tries to import connections from older version of airflow, breaking compatibility. We might need to handle that.

cc: @bugraoz93 @pierrejeambrun

Thanks for flagging @shubhamraj-git! This could indeed be a problem and needs to be handled. This will highly likely reduce the migration effort too if we have backwards compatibility and allow users to migrate their connections to v3.

TLDR airflowctl
We can parse them to JSON before sending them to the API if an empty string is passed, to provide backwards compatibility in CTL. We should warn users that an empty string for extras would end up as JSON.
I don't think this has to be included in the first release, as the airflow CLI will still support it. While deprecating duplicate functionalities, we have to implement it at least before deprecating the airflow CLI over airflowctl. It is also a small addition where we can add it. I will follow up with this

[bugraoz93]

If this won't be supported from UI, we may consider dropping the feature from airflow CLI, since if those are imported from CLI, they can end up with some API/UI problems while updates and people can think, this is allowed. If it isn't supported by the API/UI, this shouldn't be allowed from the CLI either, where we can also add small things there if it wasn't deprecated. While more isolation is ongoing, it can be easier to follow up a single source of truth for these kinds of database activities

[shubhamraj-git]

@bugraoz93 I think it would be hard to drop the functionality to support empty string, since users might have a lot of connection and might end up getting failed imports, which could be a problem. We already had some issue reported in 3.0.0, I think we can give an exception to the empty string, anyways we are supporting to edit an empty string in extra due to the issue mentioned.

[bugraoz93]

@bugraoz93 I think it would be hard to drop the functionality to support empty string, since users might have a lot of connection and might end up getting failed imports, which could be a problem. We already had some issue reported in 3.0.0, I think we can give an exception to the empty string, anyways we are supporting to edit an empty string in extra due to the issue mentioned.

I haven't meant to drop supporting it but rather storing in the database in a unified way. I support backwards compatibility importing from older versions to newer versions for a smooth transition but this doesn't necessarily mean they will be persisted as the default empty string. This means export from a newer version can work on newer versions. It could be even invisible to users if they aren't reading newsletters before updates or following up on database changes. If there is a lot of automation on it across users, we may be even more relaxed about it, I am not against that.

[shubhamraj-git]

ohh, you meant to transform the empty string to empty json upon migration and not allow creation with empty string via CLI to sync with the behaviour of UI.
Yes, you are right, this could potentially cause a problem in automations.

[jscheffl]

Coming along here after being a bit off from PC: I think only valid JSON is of any use in providers. I am not sure if an empty string is used in the providers... which would be against the idea of the extras.

To come around the problem of "legacy data" where potentially empty strings are used... I'd propose to ignore just empty strings and either convert them to {} or None implicitly. Then we can keep the validation and maybe just allow an empty string for historic reasons, post a warning and ignore the content. Support of this could be dropped in Airflow 4.

[akshayvijayjain]

@jscheffl @shubhamraj-git @bugraoz93

Thank you for inputs; what we understood is we have to allow support for '' in api to support backward compatibility for ctl.

so we should convert '' to {} before storing in DB

while from UI; we could restrict even '' | OR | to be in sync till airflow 4; we can allow '' from UI also; and since api already converting it to {}

Now; we also need to add deprecation log in

1. api call
2. CTL

Please confirm if our approach is okay!

P.S> @Prasanth345 is actively working on this ticket. I am just giving pair of eyes

[pierrejeambrun]

Yes if the problem is only accepting empty string for backward compatiblity. We can just handle it in the serializers (ConnectionResponse and ConnectionBody) with a warning / message as jens mentionned.

    :param extra: Extra metadata. Non-standard data such as private/SSH keys can be saved here. JSON
        encoded object.

Also can you add some backend test to demonstrate that we can send "" successfully.

[jason810496]

Thanks for the PR, LGTM for the API part.
Only small nits for having "" to wrap the extra` field, but no strong opinion for this, just personal preference.

[Prasanth345]

@pierrejeambrun @jason810496

Addressed backward compatibility for extra field by handling empty string ("") as {} in serializers.

Added validation to ensure extra is a valid JSON object and wrote backend tests to verify acceptance of empty string.

Also tested changes using airflow connections add

[jason810496]

Thanks for the update, it's almost ready to be merged!

Static check for the following files need fix:

• airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
• airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py

https://github.com/apache/airflow/actions/runs/16749342479/job/47415914322?pr=54034

They should be automatically fixed by running pre-commit again.

[Prasanth345]

@jason810496 fixed the static check errors

[bugraoz93]

Yes, I agree with Jens here and with comments from Pierre and Liu Zhe You. This is exactly what I mentioned: we should set it as either {} for the default empty JSON or None for sure but a unified way of handling it. I jumped in a bit later stage again but looks good, thanks for the changes and for your time!

[bugraoz93]

Looks good!

[pierrejeambrun]

manual backport #54235
cc: @potiuk

[jscheffl]

Cool! Thanks for the contribution!