Page MenuHomePhabricator

Simplify max query name length check in parse_name()
ClosedPublic

Authored by nakihito on Mar 11 2020, 23:19.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING757f9f4b9cbc: Simplify max query name length check in parse_name()
rABC757f9f4b9cbc: Simplify max query name length check in parse_name()
Summary

Rather than having two redundent checks in two different places, having
one simplier check in one place is easier to maintain and makes the
return site consistent.

Also adjusts the unit test to reflect this change. Before the change, the behavior would appear to be correct, however, the function would return an error because of the label length rather than erroring because of the length of the entire qname.

Test Plan
ninja check-bitcoin-seeder

Diff Detail

Repository
rABC Bitcoin ABC
Branch
FixParseNameMaxLengthCheck
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9791
Build 17455: Default Diff Build & Tests
Build 17454: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Mar 11 2020, 23:19
jasonbcox requested changes to this revision.Mar 12 2020, 00:06
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/seeder/test/dns_tests.cpp
134 ↗(On Diff #16887)

Don't do this. Any following tests cases that might rely on maxLengthQName will be relying on something falsely named. Make a new variable if you must.

This revision now requires changes to proceed.Mar 12 2020, 00:06

Created new variable to test an oversized qname rather than overwritting maxLegnthQName. Also edited comments for clarity.

This revision is now accepted and ready to land.Mar 12 2020, 00:22