Make dict validation stricter

[and-semakin]

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.4
            pydantic compiled: False
                 install path: /Users/andreisemakin/.local/share/virtualenvs/popapi-7C7w-VGz/lib/python3.7/site-packages/pydantic
               python version: 3.7.6 (default, Dec 30 2019, 19:38:26)  [Clang 11.0.0 (clang-1100.0.33.16)]
                     platform: Darwin-19.3.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions']

Consider following code snippet:

from pydantic import BaseModel, constr
from typing import Dict, Union


NotEmptyStr = constr(strict=True, min_length=1)


class RequestModel(BaseModel):
    value: Union[None, NotEmptyStr, Dict[str, NotEmptyStr]] = None


model = RequestModel.validate({"value": ""})

What would you expect model.value to be equal? I would expect it to raise ValidationError, because empty strings are not allowed, but apparently it coerces to dictionary:

In [4]: model.value
Out[4]: {}

[samuelcolvin]

I think a string (empty or not) should never be valid as a dict.

Only question is whether this counts as a "breaking change" and therefore needs to wait for v2?

@dmontagu @tiangolo thoughts?

[and-semakin]

Ok, then I'll split out condict part from this issue to a separate one.

[and-semakin]

More cases (continuation of previous snippet):

# list
In [5]: model2 = RequestModel.validate({"value": []})

In [6]: model2
Out[6]: RequestModel(value={})

# tuple
In [7]: model3 = RequestModel.validate({"value": ()})

In [8]: model3
Out[8]: RequestModel(value={})

# set
In [10]: model4 = RequestModel.validate({"value": set()})

In [11]: model4
Out[11]: RequestModel(value={})

# empty range
In [14]: model5 = RequestModel.validate({"value": range(100, 0)})

In [15]: model5
Out[15]: RequestModel(value={})

So I guess any empty iterable can be coerced to dictionary.

[samuelcolvin]

Yes, so can a list of pairs.

All we really do is call dict() on the thing which is not sufficient. But to block all of this would definitely be a breaking change.

[and-semakin]

It may be a perfect use case for condict(strict=True) 🤔
It won't be a breaking change, just another feature. What do you think @samuelcolvin ?

[dmontagu]

The problem is that python can and will convert any iterable to a dictionary via dict(...) if all of the items are pairs.

As a result, any iterable that has no items will be converted to {}, including the empty string.

I personally would vote to change this behavior in v2 so that anything that isn't a subclass of dict just fails the dict validator. It wouldn't surprise me if there were reasonable uses of pydantic that this would break, but I personally feel this is important enough and should break little enough code that it meets the bar for the sort of backwards compatibility breakage that we should be considering in a major version update.

I'd be curious if there are any common uses of this functionality that could pose a more serious obstacle to making this change. (For example, if we need pydantic to be able to parse a model to a Dict[str, Any] under some circumstances, or similar.)

If we are going to change this behavior in v2, I would suggest we don't add condict as a type in the main pydantic library. But @and-semakin that wouldn't stop you from making use of a custom implementation in your own code in the mean time.

Thoughts?

[and-semakin]

Connected issue: #558

[and-semakin]

Do we have any plans or estimation about when v2 will be ready?

I agree with your arguments @dmontagu. Do we want to save current behaviour of dict, maybe under some other name?

[samuelcolvin]

I agree with @dmontagu. I too thought there must be a situation where lists of pairs are coerced to a dict, but then couldn't think of one.

In terms of v2, I was originally hoping for the end of March, but I think that's no unrealistic.

Hopefully not too long through.

[spacemanspiff2007]

I have another case:

"options": [
    {
        "value": "OPEN",
        "label": "Open"
    },
    {
        "value": "CLOSED",
        "label": "Closed"
    }
]

validated with

options: Dict[str, str] = {}

becomes

{'value': 'label'}

which is quite wrong!

[samuelcolvin]

Agreed, I think for simplicity we should not allow lists as inputs to dicts in V2.

[tiangolo]

Yeah 🤔

Agreed with @dmontagu as well. ☝️

[HansBrende]

Here's a slightly less intrusive tweak that would fix many of these bugs while remaining backwards-compatible with the existing ability to coerce a dict from a list of tuples. Maybe something like this can be added sooner than v2? #2513 (comment)

[samuelcolvin]

This is fixed on main and will be included in V2, you can try it now on the alpha release.