[Freeipa-devel] [PATCH] 0050 Fail on unknown Command options

[Martin Kosek]

On Wed, 2012-05-16 at 10:37 +0200, Petr Viktorin wrote:
> On 05/16/2012 09:58 AM, Martin Kosek wrote:
> > On Tue, 2012-05-15 at 13:35 +0200, Petr Viktorin wrote:
> >> On 05/15/2012 09:55 AM, Martin Kosek wrote:
> >>> On Mon, 2012-05-14 at 14:47 +0200, Petr Viktorin wrote:
> >>>> The final part of rejecting unknown Command arguments: enable the
> >>>> validation, add tests.
> >>>> Also fix up things that were changed since the previous patches.
> >>>>
> >>>> https://fedorahosted.org/freeipa/ticket/2509
> >>>>
> >>>
> >>> The patch looks OK so far. I just found an error in permission/aci
> >>> plugin - --subtree does not work when it matches a result:
> >>>
> >>> # ipa permission-find --subtree=foo
> >>> ---------------------
> >>> 0 permissions matched
> >>> ---------------------
> >>> ----------------------------
> >>> Number of entries returned 0
> >>> ----------------------------
> >>>
> >>>    ipa permission-find
> >>> --subtree='ldap:///ipauniqueid=*,cn=hbac,dc=idm,dc=lab,dc=bos,dc=redhat,dc=Com'
> >>> ipa: ERROR: Unknown option: subtree
> >>
> >> Attaching fixed patch.
> >>
> >>> We should not pass **options to aci_show, it is too risky. There may be
> >>> other places where we don't use an option-safe approach that we want to
> >>> have fixed.
> >>
> >> We shouldn't really pass **options to any command; listing everything
> >> explicitly would be much safer. Unfortunately, in a lot of cases where
> >> commands call other commands, it's currently done this way.
> >>
> >
> > There is something wrong with the patch you sent (seen on F16 and F17):
> >
> > $ git apply ~/freeipa-pviktori-0050-02-Fail-on-unknown-Command-options.patch
> > fatal: corrupt patch at line 129
> >
> > Martin
> >
> 
> Attaching a rebased patch.
> 

Yup, this one is fine. Now, I did not find issues in the patch itself,
tests are clean.

However, thanks to this new check I found issues in Web UI (automember,
selfservice, delegation screen) which use illegal options and which
should be fixed before we push your patch:

https://fedorahosted.org/freeipa/ticket/2760

Martin