sadpandajoe commented on code in PR #45433:
URL: https://github.com/apache/superset/pull/45433#discussion_r4371528533
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -292,6 +292,278 @@ 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.
+
+ Both levels carry a hostile label, since a pivot on two dimensions can put
+ one in either. Escaping only the innermost would leave the outer one in the
+ first column of the file.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.MultiIndex.from_tuples(
+ [("=SUM(1+1)", '=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"
+ # The outer level label is escaped in the first field.
+ assert rows[1].split(",")[0] == "'=SUM(1+1)"
+ # The inner level label is escaped, pipes included.
+ assert "'=cmd" in rows[1]
+ assert rows[2] == "b,safe,2"
+
+
+def test_df_to_escaped_csv_escapes_column_axis_names():
+ """
+ The *column* axis names are escaped too.
+
+ A pivot with grouped columns writes both column axis names out as their own
+ header lines, ahead of the row labels, so a hostile warehouse column name
+ lands in the first cell of the file.
+ """
+ df = pd.DataFrame(
+ data=[[1]],
+ index=pd.Index(["row"], name="dim"),
+ columns=pd.MultiIndex.from_tuples([("outer", "v")], names=["=axis",
"+inner"]),
+ )
+
+ rows = df_to_escaped_csv(df, encoding="utf8",
index=True).strip().split("\n")
+
+ assert rows[0] == "'=axis,outer"
+ assert rows[1] == "'+inner,v"
+
+
+def test_df_to_escaped_csv_escapes_colliding_index_labels():
+ """
+ Two labels can escape to the same text, and the export still works.
+
+ ``escape_value`` is not injective: it prepends a quote, so a warehouse
+ returning both ``=a`` and ``'=a`` in one row dimension has them both land
+ on ``'=a``. Escaping ``levels`` in place rejects that with
+ ``ValueError: Level values must be unique``, which would turn a CSV export
+ into a 500, so the level is rebuilt by factorizing its row values, where a
+ repeated label is ordinary. This is the case that separates reading the row
+ values from escaping ``levels``, the sort order not being the reason.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.MultiIndex.from_tuples(
+ [("=a", "x"), ("'=a", "y")], names=["dim", "other"]
+ ),
+ )
+
+ assert (
+ df_to_escaped_csv(df, encoding="utf8", index=True)
+ == "dim,other,v\n'=a,x,1\n'=a,y,2\n"
+ )
+
+
+def test_df_to_escaped_csv_escapes_a_label_that_is_not_first_in_row_order():
+ """
+ A hostile label is escaped on its own row wherever it falls in row order.
+
+ This pins the placement, not the choice of source for the escaped level.
+ Mapping ``levels`` in place passes it too: the mapping runs position for
+ position, so the original codes go on pointing at the right entries even
+ when the hostile label sorts last. The case that separates the two is
+ ``test_df_to_escaped_csv_escapes_colliding_index_labels``, where two
+ labels escape to the same text and the level is no longer unique.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.MultiIndex.from_tuples(
+ [("safe", "x"), ("=cmd()", "y")], names=["dim", "other"]
+ ),
+ )
+
+ assert (
+ df_to_escaped_csv(df, encoding="utf8", index=True)
+ == "dim,other,v\nsafe,x,1\n'=cmd(),y,2\n"
+ )
+
+
+def
test_df_to_escaped_csv_escapes_a_flat_index_mixing_a_formula_and_non_strings():
+ """
+ A flat index holding a formula label beside non-strings exports intact.
+
+ The level is rebuilt through ``Index.map``, which hands the callable every
+ label, not just the ones needing a quote, so it has to pass a non-string
+ through untouched. ``escape_value`` on its own does not: it subscripts its
+ argument and raises ``TypeError`` on the integer. The id past 2**53 has to
+ survive the rebuild as well, and does, because the formula string keeps the
+ re-inferred dtype at ``object``.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2, 3]},
+ index=pd.Index(["=cmd()", 9007199254740993, None], name="dim"),
+ )
+
+ assert (
+ df_to_escaped_csv(df, encoding="utf8", index=True)
+ == "dim,v\n'=cmd(),1\n9007199254740993,2\n,3\n"
+ )
+
+
+def test_df_to_escaped_csv_keeps_a_numeric_index_dtype():
+ """
+ Escaping row labels does not widen a numeric row dimension.
+
+ Rebuilding the index promotes ``float32`` to ``float64``, and the wider
+ dtype prints the binary expansion of the stored value, so a label that
+ read ``1.1`` would export as ``1.100000023841858``.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.Index(np.array([1.1, 2.2], dtype="float32"), name="dim"),
+ )
+
+ assert df_to_escaped_csv(df, encoding="utf8", index=True) ==
"dim,v\n1.1,1\n2.2,2\n"
+
+
+def test_df_to_escaped_csv_keeps_a_temporal_index_dtype():
+ """
+ Escaping row labels leaves ``date_format`` working.
+
+ Rebuilding a ``datetime64`` index turns it into object dtype, and
+ ``to_csv`` only applies ``date_format`` to a datetime index, so the
+ configured ``CSV_EXPORT["date_format"]`` would quietly stop being honored.
+ """
+ df = pd.DataFrame(
+ data={"v": [1]},
+ index=pd.DatetimeIndex(
+ np.array(["1500-01-01"], dtype="datetime64[s]"), name="dim"
+ ),
+ )
+
+ escaped_csv_str = df_to_escaped_csv(
+ df, encoding="utf8", index=True, date_format="%d/%m/%Y"
+ )
+
+ assert escaped_csv_str == "dim,v\n01/01/1500,1\n"
+
+
+def test_df_to_escaped_csv_keeps_a_sibling_level_dtype():
+ """
+ A level that needs escaping is rebuilt without disturbing its siblings.
+
+ A pivot on a dimension and a time grain puts both in the index, and the
+ temporal level has to keep its dtype for ``date_format`` to reach it.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.MultiIndex.from_arrays(
+ [
+ pd.Index(["=cmd()", "safe"]),
+ pd.DatetimeIndex(
+ np.array(["1500-01-01", "1600-01-01"],
dtype="datetime64[s]")
+ ),
+ ],
+ names=["dim", "when"],
+ ),
+ )
+
+ escaped_csv_str = df_to_escaped_csv(
+ df, encoding="utf8", index=True, date_format="%d/%m/%Y"
+ )
+
+ rows = escaped_csv_str.strip().split("\n")
+
+ assert rows[1] == "'=cmd(),01/01/1500,1"
+ assert rows[2] == "safe,01/01/1600,2"
+
+
+def test_df_to_escaped_csv_keeps_a_sibling_level_dtype_with_a_missing_value():
+ """
+ A sibling level holding a missing value keeps its integer dtype.
+
+ Reading a level back row by row codes the missing value as ``-1`` and
+ widens the level to ``float64``, which writes an id past 2**53 as a
+ different number. Only the level that needs escaping is rebuilt, so the
+ others keep their own values and codes.
+ """
+ df = pd.DataFrame(
+ data={"v": [1, 2]},
+ index=pd.MultiIndex.from_tuples(
+ [("=cmd()", 9007199254740993), ("safe", None)], names=["dim", "id"]
Review Comment:
A MultiIndex level that is rebuilt and also holds a missing label is not
exercised: the missing-value cases only put the `None` in a sibling level that
is left untouched, and the sliced-out case drops the formula row before the
rebuild. If the rebuild went through `values.map(str).map(escape_value)`
instead of `values.map(escape_values)`, `MultiIndex.from_tuples([("=cmd()",
"x"), (None, "y")])` would export the second row as `nan,y,2` instead of
`,y,2`, and every test in this file would still pass. Could you add that case
and assert the second row has an empty first field?
--
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]
