aaron-y-chen commented on code in PR #71711:
URL: https://github.com/apache/airflow/pull/71711#discussion_r3943647558
##########
providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py:
##########
@@ -55,6 +55,29 @@
_K8S_WAIT_APP_COMPLETION_CONF = "spark.kubernetes.submission.waitAppCompletion"
+# Values to mask are anchored at a token boundary: without the lookbehind the
leading
+# \S*? retries at every offset in the string, which is what made masking
pathologically
+# slow on long arguments and log lines. Anchoring does not make this strictly
O(n) -- a
+# token packing many "secret"/"password" occurrences still backtracks
quadratically --
+# but it removes the retry-per-offset factor and is orders of magnitude faster
in
+# practice. A quote only closes the value when whitespace or the end of the
string
+# follows it, so quoted values may themselves contain quotes.
+_SENSITIVE_VALUE_RE = re.compile(
+ r"(?<!\S)(\S*?(?:secret|password)\S*?(?:=|\s+))"
+ r"(?:'((?:[^']|'(?!\s|$))*)'|\"((?:[^\"]|\"(?!\s|$))*)\"|(\S*))",
Review Comment:
I think the regex pattern should be modified as follows:
```suggestion
r"(?:'((?:[^'\n]|'(?!\s|$))*)'|\"((?:[^\"\n]|\"(?!\s|$))*)\"|(\S*))"
```
Otherwise, a multiline application argument can cause unrelated command-log
content to be swallowed:
```
in : 'spark-submit --conf password="abc\nERROR: job failed\n--other=1 "tail'
old: 'spark-submit --conf password=******\nERROR: job failed\n--other=1
"tail'
new: 'spark-submit --conf password="******"tail'
```
##########
providers/apache/spark/tests/unit/apache/spark/hooks/test_spark_submit.py:
##########
@@ -1315,6 +1356,35 @@ def test_masks_passwords(self, command: str, expected:
str) -> None:
# Then
assert command_masked == expected
+ @pytest.mark.db_test
+ def test_masks_passwords_stays_fast_on_large_input(self) -> None:
+ # The previous pattern retried at every offset on long inputs, taking
tens of
+ # seconds for this payload and blocking the worker slot.
+ hook = SparkSubmitHook()
+ payload = ("spark-submit", "--arg", "x " * 25_000)
Review Comment:
Why leave trailing whitespace? Should it be `"x" * 25_000`?
--
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]
