webargs: Race conditions for parallel requests due to cache

I just noticed that something in webargs or marshmallow isn’t thread-safe. Take this minimal example"

import time

from flask import Flask, jsonify, request
from marshmallow.fields import Field
from webargs.flaskparser import use_kwargs


app = Flask(__name__)


class MyField(Field):
    def _serialize(self, value, attr, obj):
        return value

    def _deserialize(self, value, attr, data):
        print 'deserialize', request.json, value
        time.sleep(0.25)
        return value


@app.route('/test', methods=('POST',))
@use_kwargs({
    'value': MyField(),
})
def test(value):
    time.sleep(1)
    return jsonify(webargs_result=value, original_data=request.json['value'])

Run it with threading enabled:

$ FLASK_APP=webargsrace.py flask run -p 8080 --with-threads

Now send two requests in parallel, with different values:

$ http post http://127.0.0.1:8080/test 'value=foo' & ; http post http://127.0.0.1:8080/test 'value=bar' &

The output from these two requests is:

{
    "original_data": "bar",
    "webargs_result": "bar"
}
{
    "original_data": "foo",
    "webargs_result": "bar"
}

Clearly not what one would have expected! 💣

The output of the print statement showing the request data and what the field receives confirms the issue:

deserialize {u'value': u'bar'} bar
deserialize {u'value': u'foo'} bar

Tested with the latest marshmallow/webargs from PyPI, and also the marshmallow3 rc (marshmallow==3.0.0rc4, webargs==5.1.2).

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 1
  • Comments: 17 (17 by maintainers)

Commits related to this issue

Most upvoted comments

I’m looking into fixing it right now

The patch is released in 5.1.3. I’ve also requested a CVE ID and will report on Tidelift once that’s done.

We can discuss refactoring the solution in https://github.com/marshmallow-code/webargs/issues/374 .

Thank you @ThiefMaster for the quick response on this.

I think this is caused by the fact that self._cache on the Parser is not thread-safe.