Skip to content

Validate Connection port is within valid TCP/UDP range (1-65535) - #70264

Closed
bujjibabukatta wants to merge 2 commits into
apache:mainfrom
bujjibabukatta:fix/#68382
Closed

bujjibabukatta wants to merge 2 commits into
apache:mainfrom
bujjibabukatta:fix/#68382

Conversation

@bujjibabukatta

Copy link
Copy Markdown
Contributor

What

The port field on the Connection REST API model (ConnectionBody) accepted
any integer, including negative numbers, 0, and values above 65535.
This adds a ge=1, le=65535 constraint so the API rejects out-of-range
port numbers with a 422 instead of silently persisting them.

Why

A TCP/UDP port is only valid in the range 1-65535. Before this change,
POST /connections and PATCH /connections would happily accept things
like port: -1 or port: 99999999, storing a value that no downstream
hook could actually connect with, and only surfacing as a confusing
runtime error much later.

How

  • airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
    — added ge=1, le=65535 to the port field on ConnectionBody. Since
    ConnectionTestRequestBody and the PATCH partial model both derive from
    ConnectionBody, this covers create, update, and connection-test paths
    with a single change.

Tests

  • test_post_should_respond_422_for_invalid_port — parametrized over
    [-1, 0, 65536, 99999, 123456789]
  • test_post_should_respond_201_for_valid_port — parametrized over
    [1, 22, 8080, 65535]
  • test_patch_should_respond_422_for_invalid_port — same invalid set via PATCH

Full existing suite (145 tests) still passes; ruff format / ruff check
clean on both changed files.

Closes: #68382

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello, Airflow's PR template contains a mandatory section regarding disclosure of AI usage which must be filled in whenever AI tooling is used (as required by the guidelines) - yet you have deliberately omitted from all of your PRs, open and closed. One of your past PRs was already flagged by a PMC member as AI-generated with unrelated changes included.

To be clear - it is allowed to use Gen-AI tools for coding, but they must be declared in the PR body.

I would like to ask you to edit the description in each of your open 4 PRs to restore the disclosure section, including filling in the checkbox and the model you used. As long as this is not addressed, the open PRs will not be merged.

Please note that if one more PR arrives with the disclosure section removed (or required details unfilled) and clear signs of AI usage, your open PRs will be closed and PMC will initiate a process for blocking your account from contributing.

@bujjibabukatta

Copy link
Copy Markdown
Contributor Author

Hi @shahar1, thanks for pointing this out.

I apologize for overlooking the AI disclosure section in the PR template. That was my mistake.

To clarify, I didn't use AI to implement the code changes. I only used it to help draft the PR description, verify the test cases locally, and review the implementation from a performance perspective. I wasn't aware that these uses also needed to be disclosed.

I'll update the descriptions of all my open PRs, restore the disclosure section, and include the model I used. I'll make sure to follow the AI disclosure guidelines properly in future contributions as well.

Thanks for bringing this to my attention, and I appreciate your patience. It won't happen again.

@shahar1

shahar1 commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

Hi @shahar1, thanks for pointing this out.

I apologize for overlooking the AI disclosure section in the PR template. That was my mistake.

To clarify, I didn't use AI to implement the code changes. I only used it to help draft the PR description, verify the test cases locally, and review the implementation from a performance perspective. I wasn't aware that these uses also needed to be disclosed.

I'll update the descriptions of all my open PRs, restore the disclosure section, and include the model I used. I'll make sure to follow the AI disclosure guidelines properly in future contributions as well.

Thanks for bringing this to my attention, and I appreciate your patience. It won't happen again.

Thanks for responding - highly appreciated, and I do apologize if I wrongly generalized.

I would really like to get your PRs merged, but it's very hard to do so when they underqualify the repo's standards + you open and immediately close them right afterwards. You won't receive much feedback this way, and it would be difficult for you to learn.

I would like to suggest maybe slowing down a bit - get a single PR standing correctly, preferrably for solving a simple issue (labeled good first issue) and without AI usage at all (with all static checks and unit tests pass). You could even reopen this one, make the adjustments, and then we could move on to the next steps.

@bujjibabukatta

Copy link
Copy Markdown
Contributor Author

Thank you @shahar1 for the thoughtful feedback and encouragement. I really appreciate you taking the time to explain the expectations.

Also, regarding the PRs I closed earlier—the reason was that I had accidentally created the branch from another feature branch instead of main. To avoid confusion and keep the history clean before making the requested changes, I closed those PRs.

Going forward, I'll slow down, focus on one PR at a time, ensure it meets the project's standards, and incorporate the feedback before moving on. Thanks again for your guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API pending-response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connection port field does not validate that the value is a valid port number

2 participants