Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made user info not required for RedisDSN. This will fix the issue #1275 #1302

Closed
wants to merge 9 commits into from
1 change: 1 addition & 0 deletions changes/1275-kesavkolla.md
@@ -0,0 +1 @@
Made user info optional for `RedisDSN`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're adding password_required and credentials_required we should put that in the change description.

2 changes: 1 addition & 1 deletion docs/examples/settings_main.py
Expand Up @@ -12,7 +12,7 @@ class Settings(BaseSettings):
auth_key: str
api_key: str = Field(..., env='my_api_key')

redis_dsn: RedisDsn = 'redis://user:pass@localhost:6379/1'
redis_dsn: RedisDsn = 'redis://:pass@localhost:6379/1'
pg_dsn: PostgresDsn = 'postgres://user:pass@localhost:5432/foobar'

special_function: PyObject = 'math.cos'
Expand Down
2 changes: 1 addition & 1 deletion docs/usage/types.md
Expand Up @@ -509,7 +509,7 @@ For URI/URL validation the following types are available:
- `AnyHttpUrl`: schema `http` or `https`, TLD not required
- `HttpUrl`: schema `http` or `https`, TLD required, max length 2083
- `PostgresDsn`: schema `postgres` or `postgresql`, userinfo required, TLD not required
- `RedisDsn`: schema `redis`, userinfo required, tld not required
- `RedisDsn`: schema `redis`, userinfo not required, tld not required
- `stricturl`, method with the following keyword arguments:
- `strip_whitespace: bool = True`
- `min_length: int = 1`
Expand Down
14 changes: 8 additions & 6 deletions pydantic/networks.py
Expand Up @@ -53,7 +53,7 @@ def url_regex() -> Pattern[str]:
if _url_regex_cache is None:
_url_regex_cache = re.compile(
r'(?:(?P<scheme>[a-z][a-z0-9+\-.]+)://)?' # scheme https://tools.ietf.org/html/rfc3986#appendix-A
r'(?:(?P<user>[^\s:/]+)(?::(?P<password>[^\s/]*))?@)?' # user info
r'(?:(?P<user>[^\s:/]+)?(?::(?P<password>[^\s/]*))?@)?' # user info
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r'(?:(?P<user>[^\s:/]+)?(?::(?P<password>[^\s/]*))?@)?' # user info
r'(?:(?P<user>[^\s:/]+)?' # user
r'(?::(?P<password>[^\s/]*))?@)?' # password

as i understand it user and password are now independent, so should go on separate lines.

r'(?:'
r'(?P<ipv4>(?:\d{1,3}\.){3}\d{1,3})|' # ipv4
r'(?P<ipv6>\[[A-F0-9]*:[A-F0-9:]+\])|' # ipv6
Expand Down Expand Up @@ -146,8 +146,9 @@ def build(
url = scheme + '://'
if user:
url += user
if password:
url += ':' + password
if password:
url += ':' + password
if user or password:
url += '@'
url += host
if port:
Expand Down Expand Up @@ -189,7 +190,8 @@ def validate(cls, value: Any, field: 'ModelField', config: 'BaseConfig') -> 'Any
raise errors.UrlSchemePermittedError(cls.allowed_schemes)

user = parts['user']
if cls.user_required and user is None:
password = parts['password']
if cls.user_required and user is None and password is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Humm, this is a breaking change (and arguably a bug) since now user is no longer required when user_required = True - just password will suffice .

To fix this, either we need to:

  1. add password_required (and then probably user_or_password_required, maybe credentials_required would be a better name)
  2. or, leave this logic unchanged and just change user_required = False on RedisDsn

I think I favour option 1., but on RedisDsn it looks like neither user or password are required, so it doesn't make much difference in this case.

raise errors.UrlUserInfoError()

host, tld, host_type, rebuild = cls.validate_host(parts)
Expand All @@ -201,7 +203,7 @@ def validate(cls, value: Any, field: 'ModelField', config: 'BaseConfig') -> 'Any
None if rebuild else url,
scheme=scheme,
user=user,
password=parts['password'],
password=password,
host=host,
tld=tld,
host_type=host_type,
Expand Down Expand Up @@ -274,7 +276,7 @@ class PostgresDsn(AnyUrl):

class RedisDsn(AnyUrl):
allowed_schemes = {'redis'}
user_required = True
user_required = False


def stricturl(
Expand Down
9 changes: 4 additions & 5 deletions tests/test_networks.py
Expand Up @@ -320,15 +320,14 @@ class Model(BaseModel):
assert m.a.user == 'user'
assert m.a.password == 'pass'

m = Model(a='redis://:pass@localhost:5432')
assert m.a == 'redis://:pass@localhost:5432'
assert m.a.password == 'pass'

with pytest.raises(ValidationError) as exc_info:
Model(a='http://example.org')
assert exc_info.value.errors()[0]['type'] == 'value_error.url.scheme'

with pytest.raises(ValidationError) as exc_info:
Model(a='redis://localhost:5432/app')
error = exc_info.value.errors()[0]
assert error == {'loc': ('a',), 'msg': 'userinfo required in URL but missing', 'type': 'value_error.url.userinfo'}


def test_custom_schemes():
class Model(BaseModel):
Expand Down