OfflineIMAP/offlineimap3

Imaputil quote function should escape backslashes first and then quotes

roboshim opened this issue · 2 comments

Hello,

there are functions quote() and dequote() in the imaputil.py. These functions should escape and unescape quotes and backslash with backslash.

def quote(s):
    s = s.replace('"', '\\"')
    s = s.replace('\\', '\\\\')
    return '"%s"' % s

def dequote(s):
    if s and s.startswith('"') and s.endswith('"'):
        s = s[1:-1]  # Strip off the surrounding quotes.
        s = s.replace('\\"', '"')
        s = s.replace('\\\\', '\\')
    return s

The function dequote() takes string, which begins and ends with quotes and removes quotes. Then, it replace all backslash+quote \" sequences with single quote " and then all sequences of two backslash \\ to one backslash \. This is correct.

The function quote() takes strings and replace all quotes " with sequence backslash+quote \" and then takes the result and replace there all backslash characters \ with sequence \\. And then return string surounded with quotes.

But is the escaping correct? I would say, the code should first replace all backslashes and then all quotes with escaped variant.

For example, if we need to quote/escape the string Folder "My data" \ 123, calling the function quote('Folder "My data" \ 123') it would first replace alle quotes with \", i.e. Folder \"My data\" \ 123, and then replace all backslashes \ with double backslash \\, i.e. Folder \\"My Data\\" \\ 123. Is this correct? I presume, it should first escape backslashes and then replace quotes, i.e. the final string should be Folder \"My data\" \\ 123. Am I right or false?

I have created PR so if you agree, please merge the PR #175

Thank you.

Regards,

Robo.

Hi,

you are right, this is the code for testing:

def quote(s):
    # s = s.replace('"', '\\"')  # Fails
    s = s.replace('\\', '\\\\')
    s = s.replace('"', '\\"')  # OK

    return '"%s"' % s


def test_quote():
    assert quote('"') == '"\\""'
    assert quote('foo"bar') == '"foo\\"bar"'


if __name__ == '__main__':
    test_quote()
    print('Test OK!')

Patch #175 applied.

Thanks!