bugfix: domain name validation.#951
Conversation
Updated regexp for domain validation Add testcases for domain validation fix issue: IdentityPython#950
tests/test_13_validate.py
Outdated
There was a problem hiding this comment.
these are domains + ports; they are not just domains.
There was a problem hiding this comment.
yep but it can be as value in DNSName? or Not?
I think it depend of setup of IdentityProvider
tests/test_13_validate.py
Outdated
tests/test_13_validate.py
Outdated
There was a problem hiding this comment.
Why should this result to an error?
There was a problem hiding this comment.
top-level domain cannot be longer than 5 characters
There was a problem hiding this comment.
top-level domain cannot be longer than 5 characters
According to the MDN, the longest a TLD can be is 63 characters. Cutting this down to a 5-character space would invalidate many top level domains, some that I own, some that I know others own.
tests/test_13_validate.py
Outdated
There was a problem hiding this comment.
domains can start with digits
There was a problem hiding this comment.
tryed to find it and I found,
I agree
src/saml2/validate.py
Outdated
There was a problem hiding this comment.
does the regex come from somewhere?
|
The original issue is due to the change from This PR introduces a new regex. I would be good to have an explanation about the regex itself. Further, we could also reuse existing packages for this purpose, like |
yep, but I found other problems with that regex. |
- Added validators library. - For domain validation uses validators.domain insted of regepx. - Updated tests according to reviews.
Here we go. |
3 participants
Updated regexp for domain validation
Add testcases for domain validation
fix issue: #950
fix small bug with domain validation