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

URL validation fails if URL is missing user field #2232

Closed
ddennerline3 opened this issue Jan 25, 2024 · 5 comments
Closed

URL validation fails if URL is missing user field #2232

ddennerline3 opened this issue Jan 25, 2024 · 5 comments

Comments

@ddennerline3
Copy link

ddennerline3 commented Jan 25, 2024

The test code below fails to parse the URL when the user field is missing, but the password is present.

from marshmallow import Schema, fields, ValidationError

def test_marshmallow_url():
    """Test marshmallow validating a URL without a username"""

    # When tracing through the code, this is the regex that was built to validate the URL 
    validate_url_regex = r"^(?:[a-z0-9\.\-\+]*)://(?:[^:@]+?(:[^:@]*?)?@|)(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|localhost|\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}|\[[A-F0-9]*:[A-F0-9:]+\]|(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.?))(?::\d+)?(?:/?|[/?]\S+)\Z"
    url_happy = "rediss://bill:[email protected]:6379"

    class UrlTest(Schema):
        my_url = fields.Url(schemes=["redis", "rediss"], require_tld=False)

    url_test = UrlTest().load({"my_url": url_happy})
    print(url_test)

    url_unhappy = "rediss://:[email protected]:6379"
    try:
        url_test = UrlTest().load({"my_url": url_unhappy})
        print(url_test)
    except ValidationError as err:
        print(url_unhappy, err)

RFC1738 specifies that either field is optional.

https://datatracker.ietf.org/doc/html/rfc1738#section-3.1

@sloria
Copy link
Member

sloria commented Feb 5, 2024

Thanks for reporting! I don't have time to dig into this right now, but would def appreciate an investigation and/or a PR from anyone who'd be so kind

@deckar01
Copy link
Member

deckar01 commented Feb 5, 2024

RFC1738 specifies that either field is optional.

A password is only optional after a user name is included.

Some or all of the parts "<user>:<password>@", ":<password>", ":<port>", and "/<url-path>" may be excluded.

password - An optional password. If present, it follows the user name separated from it by a colon.

there is no way to specify a password without specifying a user name

@sloria
Copy link
Member

sloria commented Feb 13, 2024

Oh good catch. So the current validation appears to be correct then. I think we can close this

@sloria sloria closed this as completed Feb 13, 2024
@deckar01
Copy link
Member

I started questioning why Redis would go off spec and discovered RFC1738 was the wrong spec to refer to. Redis based their URI scheme off of the NWG's 2005 RFC3986 which supersedes the 1994 RFC1738.

https://www.iana.org/assignments/uri-schemes/prov/redis

It no longer imposes restrictions on userinfo other than character set.

https://www.rfc-editor.org/rfc/rfc3986#section-3.2.1

RFC2396 dropped the "user:password" rules and left it up to the protocol.

https://datatracker.ietf.org/doc/html/rfc2396#appendix-G.2

@sloria
Copy link
Member

sloria commented Feb 26, 2024

closed by #2244

@sloria sloria closed this as completed Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants