Incorrect substitution by cursor.execute when tuple args contains '%s'

Attached is a patch I used as a work-around for the issue. It ensures the following test pass:

def test_subtargs():
    assert _subst_tuple_args( b'A%s, %sB', (b'1',b'2') ) == b'A1, 2B'
    assert _subst_tuple_args( b'A%s, %sB', (b'%s1',b'2') ) == b'A%s1, 2B'
    assert _subst_tuple_args( b"A%s, 'B%C'", (b'%s1',) ) == b"A%s1, 'B%C'"
    assert _subst_tuple_args( b"AB", () ) == b"AB"
    try:
        assert _subst_tuple_args( b"AB", (1,) ) == '', 'Too many arg exception expected'
    except ValueError:
        pass
    try:
        assert _subst_tuple_args( b'A%s, %sB', (b'1',) ) == '', 'Missing arg exception expected'
    except IndexError:
        pass

I can't reproduce the problem with the following test case (please review):

def main():
    cnx = mysql.connector.connect(database='test')
    cur = cnx.cursor()
    cur.execute('DROP TABLE IF EXISTS t1')
    cur.execute("CREATE TABLE t1 (id INT NOT NULL, c1 VARCHAR(30), c2 INT)")
    cur.execute("REPLACE INTO t1 VALUES (%s,%s,%s)", (1,'ham%s',2))
    cur.execute("SELECT * FROM t1")
    print(cur.fetchall())
    cnx.close()

The result is:

[(1, 'ham%s', 2)]

I'm reproducing the issue with the reworked sample you provided with python 3.1.

The code causing the issue is:
http://bazaar.launchpad.net/~geertjmvdk/myconnpy/main/annotate/head%3A/python3/mysql/connector/cursor.py#L307

                elif isinstance(params, (list,tuple)):
                    for p in self._process_params(params):
                        stmt = stmt.replace(b'%s',p,1)

It replaces '%s' found in the statement as many times as it there is parameters, each replacement starting over from the beginning of the statement where previous parameters' value have already been substitued. This means that if one parameter value contains '%s', this occurrence of '%s' will be considered for substitution for the next parameter...

With your test case, you end up with the following statement:
"REPLACE INTO t1 VALUES (1,'ham2',%s)"

I figured why you do not reproduce the issue with python 2.6: the implementation of cursor.execute use a very different algorithm for substitution:
http://bazaar.launchpad.net/~geertjmvdk/myconnpy/main/annotate/head%3A/python2/mysql/connector/cursor.py#L307

stmt = operation % self._process_params(params)

which does not suffer from the bug I pointed out (but potentially introduce other issues as any format escape sequence such as "%r" or "%d" would also be substitued, though it is unlikely someone stumble on this).

Seems like I did something wrong with my test case before, now I can indeed reproduce the problem.
And indeed.. it's a silly bug there.

I can't use the %s substitution like in Python 2 because I'm working with bytes in Python 3.1 code.

I think an easier fix (maybe even faster) is to first replace all %s by something else:

                elif isinstance(params, (list,tuple)):
                    stmt = stmt.replace(b'%s',b'%mys')
                    for p in self._process_params(params):
                        stmt = stmt.replace(b'%mys',p,1)

I'm using '%mys' because that would (hopefully) never or rarely clash.
What you think?

From my point of view the proposed fix only makes the bug less likely to occur, but does not fix it.

The likelihood of occurrence of '%mys' in a sequence of characters is very difficult to predict when you are storing blob/text. The proposed fix for example would fail if I were to store the python source or this bug description as text as '%mys' occurs in it. When you are storing data from the web, you want something that always works.

Also, while I did not do any performance tests, using replace without start index hint, cause the string to be rescanned from the start which can be slow when you are manipulating large text/blob.

That being said there may be a simpler fix than the one I did (though it also check if all args where used in addition to fixing the bug).

A fix is now committed and I've used the approach you used, with regular expression.
Thanks for this, I learned a few new tricks about re in Python :)