Conversation
Can't use sizof(d->d_name) Use NAME_MAX or MAXNAMLEN
|
This is a prerequisite for using System.Net.Security on illumos. |
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
There was a problem hiding this comment.
Pull request overview
Updates native X509 store enumeration to avoid relying on sizeof(dirent.d_name) (which is not usable on illumos) when allocating buffers and handling certificate file names.
Changes:
- Compute the temporary path buffer size using
NAME_MAX/MAXNAMLEN(with a fallback) instead ofsizeof(d_name). - Add a guard to skip
.pfxentries whose names won’t fit in the remaining buffer space. - Switch filename length computation from
strnlen(..., sizeof(d_name))tostrlen(...).
| } | ||
|
|
||
| // On some platforms (like illumos), d_name is declared as char[1], so sizeof(d_name) doesn't give | ||
| // the actual maximum length. Use NAME_MAX or MAXNAMLEN instead. |
There was a problem hiding this comment.
The comment mentions MAXNAMLEN, but the code doesn't. AFAICT, MAXNAMLEN is a BSD-only name, but that BSD also defines the POSIX NAME_MAX, so MAXNAMLEN should just be removed from the comment.
| // On some platforms (like illumos), d_name is declared as char[1], so sizeof(d_name) doesn't give | ||
| // the actual maximum length. Use NAME_MAX or MAXNAMLEN instead. | ||
| #ifdef NAME_MAX | ||
| static size_t maxNameLen = NAME_MAX; |
There was a problem hiding this comment.
Why is this a static rather than a #define? Given the current construction it looks like it could be a define...
| #ifdef NAME_MAX | ||
| static size_t maxNameLen = NAME_MAX; | ||
| #else | ||
| static size_t maxNameLen = 255; // Reasonable default |
There was a problem hiding this comment.
What sort of system would not define it? On that system, what is going to happen if there's no null terminator?
I think we'd be better off just using NAME_MAX, and putting in this file something like
#ifndef NAME_MAX
#error NAME_MAX is not defined, an alternative method of determining the maximum file name length is required for this platform.
#endif
Can't use sizof(d->d_name)
Use NAME_MAX or MAXNAMLEN
Without this, the code running on illumos reads only 1 character of the file names in the certificates store and cannot find any certificates.