Skip to content
New issue

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

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

Already on GitHub? # to your account

fields.URL validation error should specify "not a valid FQDN" if trying to pass hostname without domain #2243

Open
ff137 opened this issue Feb 20, 2024 · 2 comments · May be fixed by #2324
Open

Comments

@ff137
Copy link

ff137 commented Feb 20, 2024

The current validation error message: 'Not a valid URL.' is not descriptive enough to help the user, especially when they are passing a value that would appear to be a valid URL to most people, such as a docker hostname: http://test-url:8001

Sample code:

from marshmallow import Schema, fields

class TestSchema(Schema):
  url = fields.URL(required=True)

TestSchema().load({"url": "http://192.168"}) // this works
TestSchema().load({"url": "http://test-url"}) // this doesn't work
TestSchema().load({"url": "derp"}) // and this has the same error message as http://test-url

This is because fields.URL has a default: require_tld=True, for require_tld: Whether to reject non-FQDN hostnames.
FQDN = fully-qualified domain name. i.e. http://server1.example.com is a FQDN, http://server1 is not.

Suggestion: error message should be more descriptive when require_tld is True, and validation fails FQDN requirement.

@lafrech
Copy link
Member

lafrech commented Oct 18, 2024

I have no objection to this. PR welcome.

ff137 added a commit to ff137/marshmallow that referenced this issue Oct 18, 2024
Relates to marshmallow-code#2243

Signed-off-by: ff137 <ff137@proton.me>
@ff137 ff137 linked a pull request Oct 18, 2024 that will close this issue
@ff137
Copy link
Author

ff137 commented Oct 18, 2024

Hi @lafrech
Thanks - I took a swing at making a PR for it. Please take a look!

The main thing is just to improve "Not a valid URL" being a confusing message when inputting something that looks like a valid URL ...

If TLD is required, and "http://test-url" is passed, I set the error message to be: "URL must include a top-level domain (e.g., '.com', '.org')."

I wanted to include a note that it's because of the require_tld argument, which can be set to False, but obviously I don't think that's a helpful message for an end-user. Tricky balance to be helpful for developers, and not overly verbose for end-users.

So, it's just my initial attempt, and feedback is welcome!

ff137 added a commit to ff137/marshmallow that referenced this issue Oct 18, 2024
Resolves marshmallow-code#2243

Signed-off-by: ff137 <ff137@proton.me>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants