-3
\$\begingroup\$

This function formats the text in each cell of a CSV. The goal of the function is to make sure that the sequence of characters in text will never break the cell's partition. The correct output of this function looks like any of these:

  • "foo"
  • "123"
  • "drop table 'chocolate'; -- this is fine, i don't care"
  • "<script>alert(""even scripts are allowed""); /\*notice the use of double double quotes here\*/</script>"
protected string formatCSVCell(string text)
{
    // https://www.owasp.org/index.php/CSV_Injection
    // https://stackoverflow.com/questions/4617935/is-there-a-way-to-include-commas-in-csv-columns-without-breaking-the-formatting
    // http://www.creativyst.com/Doc/Articles/CSV/CSV01.htm#FileFormat
    // https://codereview.stackexchange.com/questions/201313/does-this-method-guarantee-that-cells-in-a-csv-will-be-correctly-partitioned
    var doubleQuote = "\"";

    text = text.Replace($"{doubleQuote}", $"{doubleQuote}{doubleQuote}"); // escape existing double quotes
    text = $"{doubleQuote}{text}{doubleQuote}"; // add double quotes around each value

    return text;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Can any of the three people who downvoted this explain their reasoning please. Please point to some objective violation of the site guidelines. \$\endgroup\$ – user875234 Aug 9 '18 at 18:31
  • 1
    \$\begingroup\$ This question is at the center of a discussion on meta \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Aug 9 '18 at 19:15
  • 4
    \$\begingroup\$ I personally don't think this question is off-topic, although a bit more context would be ideal, such as information about the input source and why this escaping is needed, and perhaps more code so we see this in its context in the code base. \$\endgroup\$ – Phrancis Aug 9 '18 at 19:52
  • 1
    \$\begingroup\$ The edits only focused on post formatting and changing the title of the post, other than that your description text is intact. please do not be discouraged. \$\endgroup\$ – Malachi Sep 5 '18 at 14:12
  • 1
    \$\begingroup\$ You have not edited this post since you posted it. there are quite a few new questions posed on this site daily and it takes a lot more time to give meaningful reviews than it does to give answers on some Stack Overflow questions. \$\endgroup\$ – Malachi Sep 5 '18 at 14:46
1
\$\begingroup\$

right now I can only see one thing that I might do differently, but it only shortens the code block a bit.

I might return the second statement, it removes an assignment to the variable, I don't like adding extra steps I don't have to.

protected string formatCSVCell(string text)
{
    // https://www.owasp.org/index.php/CSV_Injection
    // https://stackoverflow.com/questions/4617935/is-there-a-way-to-include-commas-in-csv-columns-without-breaking-the-formatting
    // http://www.creativyst.com/Doc/Articles/CSV/CSV01.htm#FileFormat
    // https://codereview.stackexchange.com/questions/201313/does-this-method-guarantee-that-cells-in-a-csv-will-be-correctly-partitioned
    var doubleQuote = "\"";

    text = text.Replace($"{doubleQuote}", $"{doubleQuote}{doubleQuote}"); 
    return $"{doubleQuote}{text}{doubleQuote}";
}

I would also remove those Comments because they are not useful to me.I realize that the urls are just for example

the reason I say they are not useful to me is because you have written good code here, it describes what it is doing to the point that if I had never looked at the code before (or haven't looked at it in years) I would still know what it is doing without the comments.


because it was in my head just now, and not because it is good coding practice, you could one-line this return statement and never actually assign to the input parameter.

protected string formatCSVCell(string text)
{
   var doubleQuote = "\"";

   return $"{doubleQuote}{text.Replace($"{doubleQuote}", $"{doubleQuote}{doubleQuote}")}{doubleQuote}";
}

Actually, that doesn't look as bad as I thought it was going to. Please keep in mind that I have not tested this One Liner.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.