3
\$\begingroup\$

I need to sanitize some chars. I have a solution that works but I am wondering if there is a better solution that may be faster or better or if I flat out should be approaching this differently?

function escapeStr(_str){
    if (/\"|\'|\%/g.test(_str)) {
        _str = _str.replace(/"/g, "%22");
        _str = _str.replace(/'/g, "%27");
        _str = _str.replace(/%/g, "%25");
    }
    return _str;
}

And vice versa:

function unescapeStr(_str){
    if (/\%22|\%27|\%25/g.test(_str)) {
        _str = _str.replace(/\%22/g, '"');
        _str = _str.replace(/\%27/g, "'");
        _str = _str.replace(/\%25/g, "%");
    }
    return _str;
}
Improve this question
\$\endgroup\$
6
  • \$\begingroup\$ Why the if clause? Why not just replace, even if there are no matches? \$\endgroup\$
    – abl
    Jan 27, 2017 at 22:07
  • \$\begingroup\$ Could you provide some context about what you want to accomplish? Why do you want to handle those three characters specially? \$\endgroup\$
    – 200_success
    Jan 27, 2017 at 22:11
  • 4
    \$\begingroup\$ What collation? What issues? Sounds like it could be a known problem with an existing solution. \$\endgroup\$
    – abl
    Jan 27, 2017 at 22:27
  • 3
    \$\begingroup\$ If it's telling you that replace is undefined, it's because you trying to use it when on a _str value that isn't a string. You can replace the test call with typeof _str === 'string', and it should work the same. The problem is: Why is _str sometimes not a string... Anyway, that's all irrelevant, since, as abl said, this sounds like a known problem \$\endgroup\$
    – Flambino
    Jan 28, 2017 at 0:44
  • 1
    \$\begingroup\$ Thanks all. I just learned that this is great place to get really good opinions and hints on my code. Much appreciated! \$\endgroup\$
    – Paul
    Jan 30, 2017 at 15:34

2 Answers 2

Reset to default
1
\$\begingroup\$

Instead of hardcoding the characters, and their escape values specifically in your .replace call, you can pass a callback function for the replacement. I suggested that you dive into encodeURI:

So, something like:

function escapeStr(_str) {
  return _str.replace(/([%'"])/g, encodeURI)
}

function unescapeStr(_str) {
  return _str.replace(/(%(?:2[257]))/g, decodeURI)
}
const e = escapeStr, u = unescapeStr;
console.log( e("string") );
console.log( e("some\" char%s") );
console.log( u("string") );
console.log( u("some%22 char%25s") );

Improve this answer
\$\endgroup\$
0
\$\begingroup\$

In first and second function

function escapeStr(_str){
    if (/\"|\'|\%/g.test(_str)) { // unnecessary if 
        _str = _str.replace(/"/g, "%22"); // unnecessary asigment
        _str = _str.replace(/'/g, "%27"); // unnecessary asigment
        _str = _str.replace(/%/g, "%25"); // unnecessary asigment
    }
    return _str;
}

You can add if (value) this will prevent errors while passing null of something like this

This probably more complex and optimal:

const escapeStr = str => str ? str
  .replace(/%/g, "%25")
  .replace(/"/g, "%22")
  .replace(/'/g, "%27") : str

const unescapeStr = str => str ? str
  .replace(/\%25/g, "%")  
  .replace(/\%22/g, '"')
  .replace(/\%27/g, "'") : str

And if we want to go crazy use this:

const escapeStr = str => str ? str.replace(/[%'"]/g, encodeURI) : str

const unescapeStr = str => str ? str.replace(/\(%2[257])/g, decodeURI) : str

const testData = [`"hello"`,`'world'`,`2%3`,``, `xyz`, `%22hello%22`]

for (const data of testData) {
  console.log(`${data}:`)
  console.log(`escapeStr -> ${escapeStr(data)}`)
  console.log(`escapeStr -> ${unescapeStr(data)}`)
}
Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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