Skip to content

20260409-IsValidFQDN#10183

Open
douzzer wants to merge 1 commit intowolfSSL:masterfrom
douzzer:20260409-IsValidFQDN
Open

20260409-IsValidFQDN#10183
douzzer wants to merge 1 commit intowolfSSL:masterfrom
douzzer:20260409-IsValidFQDN

Conversation

@douzzer
Copy link
Copy Markdown
Contributor

@douzzer douzzer commented Apr 9, 2026

src/internal.c: add IsValidFQDN(), and in MatchDomainName(), do pattern matching and case folding only if target string IsValidFQDN().

tested with

wolfssl-multi-test.sh ...
check-source-text
clang-tidy-all-sp-all

see #10169 and ZD #21565

Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10183

Scan targets checked: wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/internal.c
@douzzer douzzer force-pushed the 20260409-IsValidFQDN branch 2 times, most recently from dc4bd5d to 5b0f3ff Compare April 10, 2026 00:14
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐺 Skoll Code Review

Overall recommendation: APPROVE
Findings: 5 total — 5 posted, 0 skipped

Posted findings

  • [Medium] Unsigned nameSz <= 0 comparison is tautologically equivalent to == 0src/internal.c:13295
  • [Low] Header comment says 'at least two labels' but code intentionally allows single labelssrc/internal.c:13281
  • [Medium] Trailing dot stripped from str but not from pattern — asymmetric matchingsrc/internal.c:13380-13382
  • [Medium] Underscore-containing DNS names lose case-insensitive matchingsrc/internal.c:13371-13378
  • [Medium] No unit tests for IsValidFQDN or the new MatchDomainName early-return behaviorsrc/internal.c:13288-13345

Review generated by Skoll via openclaw

Comment thread src/internal.c Outdated
Comment thread src/internal.c Outdated
Comment thread src/internal.c Outdated
Comment thread src/internal.c Outdated
Comment thread src/internal.c Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

MemBrowse Memory Report

gcc-arm-cortex-m4

  • FLASH: .text +256 B (+0.1%, 195,659 B / 262,144 B, total: 75% used)

gcc-arm-cortex-m4-tls12

…idFQDN()

tests/api/test_ossl_x509.c, tests/api/test_ossl_x509.h: add test_wolfssl_local_IsValidFQDN().

src/internal.c: in MatchDomainName(), when WOLFSSL_LEFT_MOST_WILDCARD_ONLY, do pattern matching and case folding only if target string validates as an FQDN.
@douzzer douzzer force-pushed the 20260409-IsValidFQDN branch from 72a8adb to 6733e49 Compare April 21, 2026 22:03
@douzzer
Copy link
Copy Markdown
Contributor Author

douzzer commented Apr 21, 2026

retest this please
(no error log)

@douzzer douzzer requested a review from rlm2002 April 21, 2026 23:00
@douzzer douzzer removed their assignment Apr 24, 2026
@embhorn embhorn mentioned this pull request Apr 27, 2026
4 tasks
Copy link
Copy Markdown
Member

@embhorn embhorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I have a PR draft that will build on this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants