Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix GroupServ issues #432

Merged
merged 5 commits into from Jan 18, 2015

Conversation

Projects
None yet
3 participants
Contributor

shockkolate commented Jan 14, 2015

This pull request eliminates issues arising from hardcoded parsing of flags in the GroupServ flags parser. The GroupServ FLAGS help text is updated to reflect the now functional GroupServ flag +A (allows viewing of group access list). Additionally, issues with flags +* are fixed and now the behaviour is more consistent with that of ChanServ.

More details are available in the individual commit messages.

shockkolate added some commits Jan 14, 2015

@shockkolate shockkolate groupserv: Rewrite flags parser to use ga_flags
Currently the GroupServ flags parser is hardcoded for each flag. While this may
be faster than looping through ga_flags--debatable, as compilers can
probably optimize a loop over compile-time data--problems can arise from
adding a new GroupServ flag without also remembering to hardcode that
flag into the flags parser.

This patch replaces the hardcoded clauses in the flags parser switch
block with a loop across the elements of ga_flags to determine the
correct flag value for the specified flag character.
971d7a1
@shockkolate shockkolate groupserv: Fix incorrect behaviour for flags +*
In the GroupServ flags parser, asterisk is handled by setting the flags
to GA_ALL. This does not preserve flags that are not part of GA_ALL,
such as the founder flag. As a result, a sole founder of a group cannot
set flags +* on themselves.

This patch amends gs_flags_parser to preserve existing flags on +*
except for GA_BAN.
3602f61
Contributor

Renegade334 commented Jan 15, 2015

To get it closer to ChanServ, there is a pre-existing quirk that should probably be addressed: if a FLAGS command is issued on a user who is not a member of the target group, and the flags string provided results in no flags being allocated to the user (eg. FLAGS !test nick +XYZ or FLAGS !test nick -*) then the user will be added to the group but with no flags. Performing the same command a second time will remove them from the group for having no "remaining" flags.

ChanServ's FLAGS performs a check on the flagstring and fails the command if there are no valid flags. In addition, it will never leave a chanacs line with no flags.

@shockkolate shockkolate changed the title from Fix GroupServ flags parser issues to Fix GroupServ flags issues Jan 16, 2015

shockkolate added some commits Jan 16, 2015

@shockkolate shockkolate groupserv: Fix inconsistencies with FLAGS
At present, a GroupServ FLAGS command issued on an account with flags in
the group in question, without a change in flags, does not invoke
command_fail with the fault_nochange fault reason.

Additionally, if a FLAGS command is issued on an account that does not
exist in the group in question, ultimately without any flags being
applied, the account is still added to the group access list. If a FLAGS
command meeting these criteria is issued a second time on the same
account, the account is removed from the group access list as is
expected for an account no longer holding flags in the group.

This patch fixes both issues described above, the latter of which was
first addressed by @Renegade334 in #432.
3d036df
@shockkolate shockkolate Add Shockk to mailmap file 6b1de0b
@shockkolate shockkolate groupserv: Fix inconsistencies in bold text usage
This patch fixes inconsistencies in the usage of bold text wrapping
around group names in user response messages, adding the appropriate
control codes to any GroupServ user response messages that are missing
them.
f01ea8e

@shockkolate shockkolate changed the title from Fix GroupServ flags issues to Fix GroupServ issues Jan 16, 2015

@kaniini kaniini added a commit that referenced this pull request Jan 18, 2015

@kaniini kaniini Merge pull request #432 from shockkolate/fix-groupserv-flags-parser
Fix GroupServ issues
8e43b59

@kaniini kaniini merged commit 8e43b59 into atheme:master Jan 18, 2015

@shockkolate shockkolate deleted the shockkolate:fix-groupserv-flags-parser branch Jan 18, 2015

Contributor

Renegade334 commented Jan 25, 2015

@shockkolate: the same improvements/fixes should also be applied to fflags.c.

@grawity grawity pushed a commit to grawity/atheme that referenced this pull request Mar 3, 2015

@shockkolate @ilbelkyr shockkolate + ilbelkyr groupserv: Fix inconsistencies with FLAGS
At present, a GroupServ FLAGS command issued on an account with flags in
the group in question, without a change in flags, does not invoke
command_fail with the fault_nochange fault reason.

Additionally, if a FLAGS command is issued on an account that does not
exist in the group in question, ultimately without any flags being
applied, the account is still added to the group access list. If a FLAGS
command meeting these criteria is issued a second time on the same
account, the account is removed from the group access list as is
expected for an account no longer holding flags in the group.

This patch fixes both issues described above, the latter of which was
first addressed by @Renegade334 in #432.
61fe6f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment