andylamp commented on code in PR #32867:
URL: https://github.com/apache/airflow/pull/32867#discussion_r1286780909
##########
airflow/models/connection.py:
##########
@@ -43,6 +47,30 @@ def parse_netloc_to_hostname(*args, **kwargs):
return _parse_netloc_to_hostname(*args, **kwargs)
+def sanitize_conn_id(conn_id: str | None) -> str | None:
+ """
+ Sanitizes the connection id and allows only specific characters to be
within. Namely,
+ it allows alphanumeric characters plus the symbols @,#,$,%,&,!,-,_,. and
() from 1
+ and up to 200 consecutive matches.
+
+ The character selection is such that it prevents the injection of
javascript or
+ executable bits in order to avoid any awkward behavior in the front-end.
+
+ :param conn_id: The connection id to sanitize.
+ :return: the sanitized string, `None` otherwise.
+ """
+ # check if `conn_id` or our match group is `None`
+ if conn_id is None or (res := re2.match(_RE_SANITIZE_CONN_ID, conn_id)) is
None:
+ # comment, so it works...?
+ # log.warning(
+ # "We failed to match `conn_id` with value: %s to the allowed
pattern or it was None",
+ # conn_id,
+ # )
Review Comment:
Thanks for your comments, I think given that the connection class is the
ground truth for everything the original spirit of this PR was to sanitise the
`conn_id` as described in the original issue. This was meant as a good starter
issue that would be easily solved in a day or so. However, given how much back
and forth we had it seems this is more involved, especially if we need to touch
more parts of the project that I am unfamiliar with (e.g. UI code and API).
That's why I said to do the said suggested additional changes might be a larger
piece of work than I originally thought it would be.
I have pushed the code changes to remove out the commented code so the tests
pass but I am not sure if I have time to do the UI and API at the moment.
--
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]
