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

Invalid domain parser needs improvement #75

Open
rjsparks opened this issue Feb 7, 2025 · 4 comments · May be fixed by #78
Open

Invalid domain parser needs improvement #75

rjsparks opened this issue Feb 7, 2025 · 4 comments · May be fixed by #78
Assignees
@Dmutre
Labels
component: idnits3

Comments

@rjsparks
Copy link
Member

rjsparks commented Feb 7, 2025

Using https://www.ietf.org/archive/id/draft-ietf-sipcore-multiple-reasons-01.txt as input and as of:
commit 7e5802141ce21bab0ef8f01c1c035a07d92bdc34 (HEAD -> v3, ietf-tools/v3, ietf-tools/HEAD)

 % node cli.js --solarized ~/ietf/draft/draft-ietf-sipcore-multiple-reasons-01.txt
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
                      idnits ▶ 3.0.0-alpha
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀

<snip>

 2  Warning
 └- Code - INVALID_DOMAIN_TLD
 └- Desc - Domain "Q.850" has an invalid TLD.
 └- Ref  - https://www.iana.org/domains/root/db

idnits2 does not warn about this valid string that is not a domain name.
@rjsparks
Copy link
Member Author

rjsparks commented Feb 7, 2025

This also goes wrong in a large number of places in draft-ietf-tcpm-accurate-31.txt which has lines like:

   it initializes its counters to r.cep = 5, r.e0b = r.e1b = 1 and r.ceb

@rjsparks
Copy link
Member Author

rjsparks commented Feb 7, 2025

And it's misfiring on email addresses, such as these in draft-ietf-lsr-ospf-admin-tags-24.txt (where some author addresses have something.ietf@ as the username part of the email address).

Dmutre added a commit to Dmutre/idnits that referenced this issue Feb 17, 2025
@Dmutre
fix: improved fqrn domain regexp
72eb91a 
closes ietf-tools#75
@Dmutre Dmutre linked a pull request Feb 17, 2025 that will close this issue
Open
@Dmutre
Copy link
Collaborator

Dmutre commented Feb 17, 2025

I have created a new pull request to fix this. Now it doesn't recognise phrases like "Q.850" as a domain. But I don't know what to do with words like "r.ceb". Parser still adds it to domain array, because it looks like a real domain. Perhaps you have some advice?

@rjsparks
Copy link
Member Author

Restrict matches the way idnits2 does at

idnits/idnits

Lines 2729 to 2734 in 989e178

if ( fqdn !~ /([a-z0-9_-]+\.)+example(.(com|org|net))?$/ \
&& fqdn !~ /([a-z0-9_-]+\.)+(urn|uri|in-addr)\.arpa?$/ \
&& fqdn !~ /www.ietf.org/ \
&& fqdn !~ /[0-9]+\.[0-9]+\./ \
&& fqdn !~ /.\..\../ \
&& fqdn1!~ /.*@$/ ) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Dmutre Dmutre

Labels
component: idnits3
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: improved fqrn domain regexp Dmutre/idnits
2 participants
@rjsparks @Dmutre