[PATCH nft] py: fix missing decode/encode of strings

[PATCH nft] py: fix missing decode/encode of strings

[Eric Garver]

When calling ffi functions we need to convert from python strings to
utf-8. Then convert back for any output we receive.

Fixes: 586ad210368b7 ("libnftables: Implement JSON parser")
Signed-off-by: Eric Garver <eric@garver.life>
---
 py/nftables.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/py/nftables.py b/py/nftables.py
index f07163573f9a..dea417e587d6 100644
--- a/py/nftables.py
+++ b/py/nftables.py
@@ -352,9 +352,9 @@ class Nftables:
         output -- a string containing output written to stdout
         error  -- a string containing output written to stderr
         """
-        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline)
-        output = self.nft_ctx_get_output_buffer(self.__ctx)
-        error = self.nft_ctx_get_error_buffer(self.__ctx)
+        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline.encode("utf-8"))
+        output = self.nft_ctx_get_output_buffer(self.__ctx).decode("utf-8")
+        error = self.nft_ctx_get_error_buffer(self.__ctx).decode("utf-8")
 
         return (rc, output, error)
 
-- 
2.20.1

Re: [PATCH nft] py: fix missing decode/encode of strings

[Phil Sutter]

Hi,

On Wed, May 01, 2019 at 12:35:00PM -0400, Eric Garver wrote:
> When calling ffi functions we need to convert from python strings to
> utf-8. Then convert back for any output we receive.

So the problem is passing utf-8 encoded strings as command?

[...]
> -        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline)
> -        output = self.nft_ctx_get_output_buffer(self.__ctx)
> -        error = self.nft_ctx_get_error_buffer(self.__ctx)
> +        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline.encode("utf-8"))
> +        output = self.nft_ctx_get_output_buffer(self.__ctx).decode("utf-8")
> +        error = self.nft_ctx_get_error_buffer(self.__ctx).decode("utf-8")

Should the encoding be made configurable? I see encode() and decode()
parameters are optional, but as soon as I call them with a string
containing umlauts I get errors. So not sure if that would be an
alternative.

BTW, thanks for all these fixes!

Cheers, Phil

Re: [PATCH nft] py: fix missing decode/encode of strings

[Eric Garver]

On Fri, May 03, 2019 at 05:51:54PM +0200, Phil Sutter wrote:
> Hi,
> 
> On Wed, May 01, 2019 at 12:35:00PM -0400, Eric Garver wrote:
> > When calling ffi functions we need to convert from python strings to
> > utf-8. Then convert back for any output we receive.
> 
> So the problem is passing utf-8 encoded strings as command?

In python3 strings are unicode. But we need "bytes" when calling the
ctypes function since it's imported with "c_char_p". This is what
encode() is doing for us.

    https://docs.python.org/3/library/ctypes.html#fundamental-data-types

In python2 strings are a sequence of bytes already. I'll have to v2 to
if we care about python2 support.

> 
> [...]
> > -        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline)
> > -        output = self.nft_ctx_get_output_buffer(self.__ctx)
> > -        error = self.nft_ctx_get_error_buffer(self.__ctx)
> > +        rc = self.nft_run_cmd_from_buffer(self.__ctx, cmdline.encode("utf-8"))
> > +        output = self.nft_ctx_get_output_buffer(self.__ctx).decode("utf-8")
> > +        error = self.nft_ctx_get_error_buffer(self.__ctx).decode("utf-8")
> 
> Should the encoding be made configurable? I see encode() and decode()
> parameters are optional, but as soon as I call them with a string
> containing umlauts I get errors. So not sure if that would be an
> alternative.

I don't think so. Since we're calling system level stuff (nftables,
kernel) I think utf-8 is what we want.

Encoding with utf-8 does the right thing:

python3:

    >>> "ö".encode("utf-8")
    >>> b'\xc3\xb6'

python2:

    >>> u"ö".encode("utf-8")
    >>> '\xc3\xb6'