Handle posix hostname edge cases #14
Conversation
|
Thank you for this PR, changes look solid!
Would it also be possible to cover that case with the test? It, of course, looks like a platform-dependant change due to dependency on |
src/nix.rs
Outdated
| unsafe { libc::sysconf(libc::_SC_HOST_NAME_MAX) as libc::size_t }; | ||
| let limit = | ||
| unsafe { libc::sysconf(libc::_SC_HOST_NAME_MAX) }; | ||
| let size = if limit < 0 { 255 } else { limit } as usize; |
There was a problem hiding this comment.
I'm assuming that this change goes from the following statement?
Max length of a hostname, not including the terminating null byte, as returned by
gethostname(2). Must not be less than_POSIX_HOST_NAME_MAX(255).
It seems that _POSIX_HOST_NAME_MAX is not defined in libc crate, so that's why it is hardcoded as a simple 255 here?
There was a problem hiding this comment.
That's correct. Unfortunately I did not find an already defined constant anywhere.
There was a problem hiding this comment.
Indeed, it is not defined anywhere for some (?) reasons. Do you think it would be better define it here locally for better readability and understanding of what is happening exactly?
Ex.
const _POSIX_HOST_NAME_MAX = 255;I'm also looking at the documentation here
`HOST_NAME_MAX` - `_SC_HOST_NAME_MAX` Maximum length of a hostname, not including the terminating null byte, as returned by `gethostname(2)`. Must not be less than `_POSIX_HOST_NAME_MAX` (255).
Perhaps we should do smth like max(limit, _POSIX_HOST_NAME_MAX) instead to fully conform the specification?
Please keep in mind that it is just a suggestion, discussions are more that welcomed :)
There was a problem hiding this comment.
I thought about something like that as well. We could still run into issues if sysconf returns a huge value (larger than stack), but on the other hand the upper limit seems to be kind of fixed. Maybe a constant "255" without even asking sysconf is even easier...
I have adjusted the pull request. Let me know if it fits your style requirements and ideas.
HOST_NAME_MAX is the maximum allowed hostname excluding the terminating nul byte. This means that HOST_NAME_MAX + 1 must be used for gethostname call. Otherwise -1 may be returned, e.g. by glibc's gethostname. How to reproduce on Linux with glibc: - Set hostname to a 64 character long string - Try to retrieve hostname
Generally yes, but it would imply that we modify the hostname. This in turn means that we would need proper permissions while running the tests. Or it would take some form of mocking. Not sure if it's worth it (or how to get the mocking done with Rust, to be honest). |
If sysconf returns -1 this either means that an error occurred or that no limit exists. Casting -1 to size_t leads to a huge number, so only use the returned limit if it is not -1. If sysconf returned an error or no limit exists, use 255.
Invalid hostnames supplied to set should be rejected. Otherwise it could happen that on FreeBSD a partial hostname is set.
Proof of Concept: If you set your hostname to a 64 character long string in Linux then a test fails. :)
Shoutout to @c3h2_ctf