Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding url sanitisation for extra links #41665

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

amoghrajesh
Copy link
Contributor

Adding url sanitisation for the extra links that can be added and consumed as plugins.
The idea here is to disable the button if there's some violation and the url turns out to be un sanitised.
Before:
image

After:
image


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Aug 22, 2024
@potiuk potiuk merged commit 6c463b3 into apache:main Aug 22, 2024
52 checks passed
@bbovenzi
Copy link
Contributor

Good catch! @amoghrajesh can you cherry pick this for v-2-10-test too?

@potiuk
Copy link
Member

potiuk commented Aug 22, 2024

Oh yeah. My Bad not asking for that.

@potiuk potiuk added this to the Airflow 2.10.1 milestone Aug 22, 2024
@potiuk
Copy link
Member

potiuk commented Aug 22, 2024

Marked it as 2.10.1 just in case we miss it accidentally. Also see #41457 (comment) - we should mark all such PRs as Milestone 2.10.x and get a final check at release time if all such changes have been merged - otherwise - we can miss some of those

@amoghrajesh
Copy link
Contributor Author

Good catch! @amoghrajesh can you cherry pick this for v-2-10-test too?

Ah thanks. Still getting used to this!

amoghrajesh added a commit to amoghrajesh/airflow that referenced this pull request Aug 22, 2024
@amoghrajesh
Copy link
Contributor Author

@bbovenzi @potiuk here's the cherry pick #41680
@potiuk do we ask users to git cherry-pick -x <hash>? It is easier to track history with the same hash, or it makes it harder in the long term

potiuk pushed a commit that referenced this pull request Aug 22, 2024
@utkarsharma2 utkarsharma2 added the type:bug-fix Changelog: Bug Fixes label Aug 30, 2024
utkarsharma2 pushed a commit that referenced this pull request Sep 2, 2024
utkarsharma2 added a commit that referenced this pull request Sep 4, 2024
utkarsharma2 added a commit to astronomer/airflow that referenced this pull request Sep 4, 2024
In the initial PR(apache#41665) we didn't handle the relative path in URL which led to issue(apache#41977). This PR aims at handling the relative path case when sanitizing URLs
utkarsharma2 added a commit that referenced this pull request Sep 4, 2024
* Handle relative paths when sanitizing URLs

In the initial PR(#41665) we didn't handle the relative path in URL which led to issue(#41977). This PR aims at handling the relative path case when sanitizing URLs

* Add PR suggestions

* Update code comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants