check for duplicate ACL entries when applying listen.acl_* #18362
Conversation
….acl_groups otherwise FPM fails to start with `Invalid argument (22)`
There was a problem hiding this comment.
Sorry for slight delay in review.
I think the logic makes sense (except the few mentioned issues).
It would be also good to add a test for this. We got some basic ACL tests but this might slightly more complex. If it's too hard for you, let me know I will try to come up with something.
| return -1; | ||
| } | ||
| if (tagconf == ACL_USER) { | ||
| uidconf = acl_get_qualifier(entryfile); |
There was a problem hiding this comment.
The entryfile is not initialized so I assume you wanted to use entryconf here, right?
| return -1; | ||
| } | ||
| } else { | ||
| gidconf = acl_get_qualifier(entryfile); |
| @@ -300,7 +303,67 @@ int fpm_unix_set_socket_permissions(struct fpm_worker_pool_s *wp, const char *pa | |||
| return -1; | |||
| } | |||
| /* Copy the new ACL entry from config */ | |||
| for (i=ACL_FIRST_ENTRY ; acl_get_entry(aclconf, i, &entryconf) ; i=ACL_NEXT_ENTRY) { | |||
| for (iconf=ACL_FIRST_ENTRY ; acl_get_entry(aclconf, iconf, &entryconf) ; iconf=ACL_NEXT_ENTRY) { | |||
There was a problem hiding this comment.
NIT: might be good to fix the CS when you are in it:
| for (iconf=ACL_FIRST_ENTRY ; acl_get_entry(aclconf, iconf, &entryconf) ; iconf=ACL_NEXT_ENTRY) { | |
| for (iconf = ACL_FIRST_ENTRY; acl_get_entry(aclconf, iconf, &entryconf); iconf = ACL_NEXT_ENTRY) { |
| return -1; | ||
| } | ||
| } | ||
| for (ifile=ACL_FIRST_ENTRY ; acl_get_entry(aclfile, ifile, &entryfile) ; ifile=ACL_NEXT_ENTRY) { |
There was a problem hiding this comment.
NIT: another CS style fix
| for (ifile=ACL_FIRST_ENTRY ; acl_get_entry(aclfile, ifile, &entryfile) ; ifile=ACL_NEXT_ENTRY) { | |
| for (ifile = ACL_FIRST_ENTRY; acl_get_entry(aclfile, ifile, &entryfile); ifile = ACL_NEXT_ENTRY) { |
|
btw. this was just an initial look (needed to also spent a bit time to get myself a bit more familiar with ACL) so will need check it out more and do some testing later to see if it all works fine. Well, ideally we want to have a working test for this. |
otherwise FPM fails to start with
Invalid argument (22)fixes GH-18357