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.

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

fix: domain name length #1965

Merged
merged 1 commit into from
Nov 30, 2022
Merged

Conversation

wayslog
Copy link

@wayslog wayslog commented Oct 26, 2022

What problem does this PR solve?

Issue Number: #1911

Problem Summary: hostname2endpoint implement is not compatite with domain RFCs.

What is changed and the side effects?

Changed: change the buf length as 256, and use MAX_DOMAIN_LENGTH to check the domain name valid. But I don't have an exists domain to test it.

Side effects:

  • Performance effects(性能影响): hostname2endpoint will allocate more 192 byte memory in stack.

  • Breaking backward compatibility(向后兼容性): yes


Check List:

  • Please make sure your changes are compilable(请确保你的更改可以通过编译).
  • When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
  • Please follow Contributor Covenant Code of Conduct.(请遵循贡献者准则).

// and RFC 5892. The full domain name may not exceed the length of 253 characters in its textual representation
// (Domain Names - Domain Concepts and Facilities. IETF. doi:10.17487/RFC1034. RFC 1034.).
// For cacheline optimize, use buf size as 256;
char buf[256];
Copy link
Contributor

Choose a reason for hiding this comment

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

建议加个static_assert
MAX_DOMAIN_LENGTH < sizeof(buf)

@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 26, 2022

建议补充一下边界值的单测

@wayslog
Copy link
Author

wayslog commented Oct 27, 2022

建议补充一下边界值的单测

这里有个麻烦的地方在于我没有一个可用的足够长的域名,导致我的返回值无论正确错误都是 -1 的。
这个地方是否可以考虑开个特例,比如参数不对的时候返回 -2 什么的,这样好做测试?

@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 27, 2022

建议补充一下边界值的单测

这里有个麻烦的地方在于我没有一个可用的足够长的域名,导致我的返回值无论正确错误都是 -1 的。 这个地方是否可以考虑开个特例,比如参数不对的时候返回 -2 什么的,这样好做测试?

可以,或者返回-1,并设置errno

@serverglen
Copy link
Contributor

@wayslog remind

@serverglen serverglen added the bug the code does not work as expected label Nov 30, 2022
@lorinlee lorinlee merged commit 01474e5 into apache:master Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug the code does not work as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants