You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "shashbha14 (via GitHub)" <gi...@apache.org> on 2026/05/08 11:39:31 UTC

[PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

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

   Closes #66591
   In 'ConnectionForm.tsx', line 85, 'JSON.parse(initialConnection.extra)' is invoked in a useEffect hook without any error handling. As a result, if any connection has some bad JSON in its extra property, that will throw an exception and stop Edit Connection form from opening.
   This is done with a try-catch statement on lines 103-110 in the same file. Applied the exact same approach – in case the JSON parse failed, fall back to the string value so that the form can open successfully.
   > I have used Claude (claude.ai) for this implementation part.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4406939749

   I'd like to reproduce this issue locally.
   Do you happen to have a screen recording showing the behavior before and after the fix?


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "shashbha14 (via GitHub)" <gi...@apache.org>.
shashbha14 commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4421945459

   > Code looks good to me, but I think this would be difficult to reproduce through normal usage, since the invalid JSON has to be inserted directly into the DB.
   > 
   > @shashbha14 If you still have the reproduction scenario, would you mind sharing it? I think that context could be helpful for reviewers.
   
   This can happen in a few real-world scenarios beyond direct DB edits:
   1. Connections migrated from older Airflow versions where validation was less strict
   2. Connections created via the REST API or CLI without JSON validation
   3. Connections imported from external systems or configuration management  tools that store extra as a plain string
   The fix is a defensive guard consistent with what the same file already does at lines 103-110, so even if the scenario is rare it is the correct pattern to follow.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "shashbha14 (via GitHub)" <gi...@apache.org>.
shashbha14 commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4408510904

   > I'd like to reproduce this issue locally. Do you happen to have a screen recording showing the behavior before and after the fix?
   
   I don't have a screen recording, but here are the steps to reproduce locally:
   
   1. Directly insert a connection with invalid JSON in the extra field via the DB:
      UPDATE connection SET extra = 'invalid json' WHERE conn_id = 'your_conn';
   2. Open the Airflow UI → Admin → Connections
   3. Click the Edit button on that connection
   4. Without the fix: the form fails to render
   5. With the fix: the form opens and shows the raw string
   
   The root cause is visible in the code — JSON.parse on line 85 has no 
   try-catch, while the same file already handles this at lines 103-110.
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4434388227

   ### Backport successfully created: v3-2-test
   
   Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
   the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
   
   In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
   
   <table>
           <tr>
           <th>Status</th>
           <th>Branch</th>
           <th>Result</th>
       </tr>
       <tr>
           <td>✅</td>
           <td>v3-2-test</td>
           <td><a href="https://github.com/apache/airflow/pull/66831"><img src="https://img.shields.io/badge/PR-66831-blue" alt="PR Link"></a></td>
       </tr>
   </table>


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi merged PR #66593:
URL: https://github.com/apache/airflow/pull/66593


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "shashbha14 (via GitHub)" <gi...@apache.org>.
shashbha14 commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4427759627

   > Thanks for the detailed context. I might be missing something, but I'm having trouble reproducing this against current main.
   > 
   > From what I can tell:
   > 
   > 1. The REST API already rejects invalid JSON in `ConnectionBody.validate_extra`.
   > 2. airflowctl `connections import/add` go through the same validation path.
   > 3. `Connection._validate_extra()` raises during `get_extra()`, so even legacy invalid rows should fail before reaching `ConnectionResponse.redact_extra()`
   > 
   > So if invalid JSON actually made it into the DB, I'd expect the connections API itself to 500 before the Edit dialog receives data.
   > 
   > That said, I'm still 👍 on merging this as defense-in-depth since the change is tiny and consistent
   > 
   > If you were able to reproduce the form crash on a 3.x setup, I'd also be very interested in a screenshot or recording? since that would suggest a serializer/API issue beyond this PR alone.
   
   Fair point, the validation guards make this hard to trigger in practice. 
   It's just a small safety net matching what's already done at lines 103-110. 
   Happy to go with whatever you decide!


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4422546899

   Thanks for the detailed context. I might be missing something, but I'm having trouble reproducing this against current main.
   
   From what I can tell:
   1. The REST API already rejects invalid JSON in `ConnectionBody.validate_extra`.
   2. airflowctl `connections import/add` go through the same validation path.
   3. `Connection._validate_extra()` raises during `get_extra()`, so even legacy invalid rows should fail before reaching `ConnectionResponse.redact_extra()`
   
   So if invalid JSON actually made it into the DB, I'd expect the connections API itself to 500 before the Edit dialog receives data.
   
   That said, I'm still 👍 on merging this as defense-in-depth since the change is tiny and consistent
   
   If you were able to reproduce the form crash on a 3.x setup, I'd also be very interested in a screenshot or recording?
   since that would suggest a serializer/API issue beyond this PR alone.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix ConnectionForm crashing when connection has invalid extra JSON [airflow]

Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on PR #66593:
URL: https://github.com/apache/airflow/pull/66593#issuecomment-4420601492

   @choo121600 - were you able to reproduce this on your end?


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org