sadpandajoe commented on code in PR #45433:
URL: https://github.com/apache/superset/pull/45433#discussion_r4320668563
##########
superset/utils/csv.py:
##########
@@ -115,6 +115,18 @@ def escape_values(v: Any) -> Union[str, Any]:
if isinstance(value, str):
df.at[label, name] = escape_value(value)
+ # Row labels and axis names are written out whenever the caller asks for
the
+ # index, which both export paths do once post-processing has replaced the
+ # range index: pivot_table_v2 promotes the row dimension's warehouse values
+ # into index labels and the dimension name into ``index.name``. Escaping
+ # happens after the cell loop so the labels ``.at`` addresses stay the ones
+ # the frame was handed over with. ``rename`` applies the mapper to every
+ # level of a MultiIndex; the axis names are a separate attribute, hence the
+ # ``rename_axis``. This mirrors the four places
+ # ``superset.utils.excel.quote_formulas`` covers for the xlsx writer.
+ df = df.rename(index=escape_values)
Review Comment:
Callable `rename(index=...)` rebuilds the index even when no label changes,
so non-string indexes can export differently than before. With pandas 2.3.3 a
`float32` row dimension (`1.1`, `2.2`) now writes `1.100000023841858` /
`2.200000047683716`, and a `datetime64[s]` index in year 1500 turns into object
dtype, so `CSV_EXPORT["date_format"] = "%d/%m/%Y"` is ignored and the row
writes `1500-01-01 00:00:00` instead of `01/01/1500`. Both frames come out
unchanged on the base. Would it make sense to only rename index levels that can
hold strings (object/string dtype), so numeric and datetime indexes keep their
dtype?
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -278,6 +278,63 @@ def test_df_to_escaped_csv_non_default_index():
assert rows[2] == ["girl Mary", "safe"]
+def test_df_to_escaped_csv_escapes_index_labels():
+ """
+ Formulas in row labels are escaped too, not just cells and headers.
+
+ Pivot post-processing promotes the row dimension's values into the index,
+ and both export paths write the index out once it is no longer a
+ RangeIndex, so a hostile warehouse string can reach the first column of
+ the file.
+ """
+ df = pd.DataFrame(
+ data={"SUM(num)": [10.0, 20.0]},
+ index=pd.Index(['=cmd|" /C calc"!A0', "safe"], name="dim"),
+ )
+
+ rows = df_to_escaped_csv(df, encoding="utf8",
index=True).strip().split("\n")
+
+ assert rows[1].startswith("\"'=cmd")
+ assert rows[2].startswith("safe,")
+ # The caller's frame is left alone.
+ assert df.index.tolist() == ['=cmd|" /C calc"!A0', "safe"]
+
+
+def test_df_to_escaped_csv_escapes_multiindex_labels_and_axis_names():
+ """
+ Every level of a MultiIndex is escaped, and so are the axis *names*.
+
+ Axis names are a separate attribute from the labels, so ``rename`` alone
+ leaves them untouched while pandas still writes them out as header cells.
+ A pivoted group-by puts a warehouse-controlled column name there.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.MultiIndex.from_tuples(
+ [("a", '=cmd|" /C calc"!A0'), ("b", "safe")],
+ names=["level", "+name"],
+ ),
+ )
+
+ rows = df_to_escaped_csv(df, encoding="utf8",
index=True).strip().split("\n")
+
+ # Header row carries the escaped axis name.
+ assert rows[0] == "level,'+name,v"
Review Comment:
This test would still pass if `columns=escape_values` were dropped from the
`rename_axis` call: only index names are asserted, but a pivot with MultiIndex
columns and `index=True` writes the column axis names into the header too
(`=axis,outer` / `+inner,v` unescaped). Could you add a case with
`columns=pd.MultiIndex.from_tuples([("outer", "v")], names=["=axis",
"+inner"])` and assert the first two lines are `'=axis,outer` and `'+inner,v`?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
