barrysteyn commented on issue #34093:
URL: https://github.com/apache/airflow/issues/34093#issuecomment-1706773954
Thanks @potiuk. The ultimate aim of `allowed_deserialization_classes` is
provide protection against unexpected class deserialization. Using a regex is
goes against that - as @bolkedebruin pointed out, you can have unexpected
results and end up allowing things you didn't want. However, I am guessing one
of the things that @bolkedebruin wanted to do was to switch things off
completely by allowing the regex `.*` which would match everything.
That said, I think we should do the following:
1. If `allowed_deserialization_classes` is None, then operate as per normal
(which would be not to allow cusomt class deserialization)
2. If `allowed_deserialization_classes` is `''` (empty string), turn the
feature off (I am still not sure how useful this feature is, but I bet there
are lots of people who would like to turn it off).
3. If `allowed_deserialization_classes` has strings in it, then use
[fnmatch](https://docs.python.org/3/library/fnmatch.html#fnmatch.filter) to do
(as you suggest) a **glob** like pattern matching search.
4. Do not do any regex matching (and since #28829, this feature has been
broken and so I don't think it is used much - except maybe to turn things off)
A note on (4) above: I do not advocate for trying string match, then glob
match and then regex. This is way too complex for what is aimed at securing the
system. It opens up a can of worms. Rather, let's just use `fnmatch` if we need
to match. And... we need to vastly improve the documentation:
1. Remove cryptic comments like `Bare “.” will be replaced so you can set
airflow*` - I am a first language English speaker and I have no idea what that
implies.
2. State that `allowed_deserialization_classes` is just a space separated
string that will be used with `split()` to make lists.
3. Give the rules as I mention above (`None` will enforce things, `''`
will stop enforcement, all other strings will be split and matched with
`fnmatch`)
What do you think?
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
