2019-03-26 14:11:14

by Kenneth Dsouza

Subject: [PATCH] nfs4_setfacl: Skip comment field while reading ACE(s).

With commit 6630629bb661a7f48fb9856f7fd9616ce1499efa an additional field for filename
was added due to which nfs4_setfacl failed to handle comments while reading ACE(s)
from nfs4_getfacl output.
This patch resolves the issue by skipping comment header.

With fix:

$ nfs4_setfacl --test -s "$(nfs4_getfacl file1)" file2
Skipping comment # file: file1
## Test mode only - the resulting ACL for "/test/file2":
A::OWNER@:rwatTcCy
A:g:GROUP@:rtcy
A::EVERYONE@:rtcy

Without fix:

$ nfs4_setfacl --test -s "$(nfs4_getfacl file1)" file2
Failed while inserting ACE(s).

Signed-off-by: Kenneth D'souza <[email protected]>
---
libnfs4acl/nfs4_insert_string_aces.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/libnfs4acl/nfs4_insert_string_aces.c b/libnfs4acl/nfs4_insert_string_aces.c
index 5a482d5..50b7bbf 100644
--- a/libnfs4acl/nfs4_insert_string_aces.c
+++ b/libnfs4acl/nfs4_insert_string_aces.c
@@ -45,21 +45,25 @@ int nfs4_insert_string_aces(struct nfs4_acl *acl, const char *acl_spec, unsigned
if ((s = sp = strdup(acl_spec)) == NULL)
goto out_failed;

+
while ((ssp = strsep(&sp, ",\t\n\r")) != NULL) {
if (!strlen(ssp))
continue;

- if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
- goto out_failed;
+ if(*ssp == '#')
+ printf("Skipping comment %s\n", ssp);
+ else {
+ if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
+ goto out_failed;

- if (nfs4_insert_ace_at(acl, ace, index++)) {
- free(ace);
- goto out_failed;
+ if (nfs4_insert_ace_at(acl, ace, index++)) {
+ free(ace);
+ goto out_failed;
+ }
}
}
if (acl->naces == 0)
goto out_failed;
-
out:
if (s)
free(s);
--
2.20.1

2019-04-09 20:50:12

by J. Bruce Fields

Subject: Re: [PATCH] nfs4_setfacl: Skip comment field while reading ACE(s).

Applied, with some fixes:

On Tue, Mar 26, 2019 at 07:41:09PM +0530, Kenneth D'souza wrote:
> With commit 6630629bb661a7f48fb9856f7fd9616ce1499efa an additional field for filename
> was added due to which nfs4_setfacl failed to handle comments while reading ACE(s)
> from nfs4_getfacl output.
> This patch resolves the issue by skipping comment header.
>
> With fix:
>
> $ nfs4_setfacl --test -s "$(nfs4_getfacl file1)" file2
> Skipping comment # file: file1

Just skip it silently, there's not need to print anything.

> ## Test mode only - the resulting ACL for "/test/file2":
> A::OWNER@:rwatTcCy
> A:g:GROUP@:rtcy
> A::EVERYONE@:rtcy
>
> Without fix:
>
> $ nfs4_setfacl --test -s "$(nfs4_getfacl file1)" file2
> Failed while inserting ACE(s).
>
> Signed-off-by: Kenneth D'souza <[email protected]>
> ---
> libnfs4acl/nfs4_insert_string_aces.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/libnfs4acl/nfs4_insert_string_aces.c b/libnfs4acl/nfs4_insert_string_aces.c
> index 5a482d5..50b7bbf 100644
> --- a/libnfs4acl/nfs4_insert_string_aces.c
> +++ b/libnfs4acl/nfs4_insert_string_aces.c
> @@ -45,21 +45,25 @@ int nfs4_insert_string_aces(struct nfs4_acl *acl, const char *acl_spec, unsigned
> if ((s = sp = strdup(acl_spec)) == NULL)
> goto out_failed;
>
> +
> while ((ssp = strsep(&sp, ",\t\n\r")) != NULL) {
> if (!strlen(ssp))
> continue;
>
> - if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
> - goto out_failed;
> + if(*ssp == '#')
> + printf("Skipping comment %s\n", ssp);

So let's just do a "continue" here, as above. Also notice we usually
leave a space after "if".

--b.

> + else {
> + if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
> + goto out_failed;
>
> - if (nfs4_insert_ace_at(acl, ace, index++)) {
> - free(ace);
> - goto out_failed;
> + if (nfs4_insert_ace_at(acl, ace, index++)) {
> + free(ace);
> + goto out_failed;
> + }
> }
> }
> if (acl->naces == 0)
> goto out_failed;
> -
> out:
> if (s)
> free(s);
> --
> 2.20.1

2019-04-10 09:28:24

by Kenneth Dsouza

Subject: Re: [PATCH] nfs4_setfacl: Skip comment field while reading ACE(s).

Thanks for taking care of this.

On Wed, Apr 10, 2019 at 2:20 AM J. Bruce Fields <[email protected]> wrote:
>
> Applied, with some fixes:
>
> On Tue, Mar 26, 2019 at 07:41:09PM +0530, Kenneth D'souza wrote:
> > With commit 6630629bb661a7f48fb9856f7fd9616ce1499efa an additional field for filename
> > was added due to which nfs4_setfacl failed to handle comments while reading ACE(s)
> > from nfs4_getfacl output.
> > This patch resolves the issue by skipping comment header.
> >
> > With fix:
> >
> > $ nfs4_setfacl --test -s "$(nfs4_getfacl file1)" file2
> > Skipping comment # file: file1
>
> Just skip it silently, there's not need to print anything.
>
> > ## Test mode only - the resulting ACL for "/test/file2":
> > A::OWNER@:rwatTcCy
> > A:g:GROUP@:rtcy
> > A::EVERYONE@:rtcy
> >
> > Without fix:
> >
> > $ nfs4_setfacl --test -s "$(nfs4_getfacl file1)" file2
> > Failed while inserting ACE(s).
> >
> > Signed-off-by: Kenneth D'souza <[email protected]>
> > ---
> > libnfs4acl/nfs4_insert_string_aces.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/libnfs4acl/nfs4_insert_string_aces.c b/libnfs4acl/nfs4_insert_string_aces.c
> > index 5a482d5..50b7bbf 100644
> > --- a/libnfs4acl/nfs4_insert_string_aces.c
> > +++ b/libnfs4acl/nfs4_insert_string_aces.c
> > @@ -45,21 +45,25 @@ int nfs4_insert_string_aces(struct nfs4_acl *acl, const char *acl_spec, unsigned
> > if ((s = sp = strdup(acl_spec)) == NULL)
> > goto out_failed;
> >
> > +
> > while ((ssp = strsep(&sp, ",\t\n\r")) != NULL) {
> > if (!strlen(ssp))
> > continue;
> >
> > - if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
> > - goto out_failed;
> > + if(*ssp == '#')
> > + printf("Skipping comment %s\n", ssp);
>
> So let's just do a "continue" here, as above. Also notice we usually
> leave a space after "if".
>
> --b.
>
> > + else {
> > + if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
> > + goto out_failed;
> >
> > - if (nfs4_insert_ace_at(acl, ace, index++)) {
> > - free(ace);
> > - goto out_failed;
> > + if (nfs4_insert_ace_at(acl, ace, index++)) {
> > + free(ace);
> > + goto out_failed;
> > + }
> > }
> > }
> > if (acl->naces == 0)
> > goto out_failed;
> > -
> > out:
> > if (s)
> > free(s);
> > --
> > 2.20.1