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.

Sign up for GitHub

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

Jump to bottom

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

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

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

Comments

@ff137
Copy link

ff137 commented Feb 20, 2024 •
edited
Loading

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
@ff137
✨ Improve error messages for URL validation
5bfac58 
Relates to marshmallow-code#2243

Signed-off-by: ff137 <[email protected]>
@ff137 ff137 linked a pull request Oct 18, 2024 that will close this issue
Open
@ff137
Copy link
Author

ff137 commented Oct 18, 2024 •
edited
Loading

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
@ff137
✨ Improve error messages for URL validation
67142cf 
Resolves marshmallow-code#2243

Signed-off-by: ff137 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

✨ Improve error messages for URL validation ff137/marshmallow
2 participants
@lafrech @ff137