[ovs-dev] Un-revert Work around Python/C JSON unicode differences
diff mbox series

Message ID 1547475336-17434-1-git-send-email-twilson@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev] Un-revert Work around Python/C JSON unicode differences
Related show

Commit Message

Terry Wilson Jan. 14, 2019, 2:15 p.m. UTC
This fix was reverted because it depended on a small bit of code
in a patch that was reverted that changed some python/ovs testing
and build. The fix is still necessary.

The OVS C-based JSON parser operates on bytes, so the parser_feed
function returns the number of bytes that are processed. The pure
Python JSON parser currently operates on unicode, so it expects
that Parser.feed() returns a number of characters. This difference
leads to parsing errors when unicode characters are passed to the
C JSON parser from Python.

Signed-off-by: Terry Wilson <twilson@redhat.com>
---
 python/ovs/json.py    | 10 ++++++----
 python/ovs/jsonrpc.py |  9 +++++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

Comments

Lucas Alvares Gomes Jan. 15, 2019, 6:18 p.m. UTC | #1
On Mon, Jan 14, 2019 at 2:35 PM Terry Wilson <twilson@redhat.com> wrote:
>
> This fix was reverted because it depended on a small bit of code
> in a patch that was reverted that changed some python/ovs testing
> and build. The fix is still necessary.
>
> The OVS C-based JSON parser operates on bytes, so the parser_feed
> function returns the number of bytes that are processed. The pure
> Python JSON parser currently operates on unicode, so it expects
> that Parser.feed() returns a number of characters. This difference
> leads to parsing errors when unicode characters are passed to the
> C JSON parser from Python.
>
> Signed-off-by: Terry Wilson <twilson@redhat.com>
> ---
>  python/ovs/json.py    | 10 ++++++----
>  python/ovs/jsonrpc.py |  9 +++++++--
>  2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/python/ovs/json.py b/python/ovs/json.py
> index e84063f..f06204e 100644
> --- a/python/ovs/json.py
> +++ b/python/ovs/json.py
> @@ -21,10 +21,13 @@ import sys
>
>  import six
>
> +PARSER_C = 'C'
> +PARSER_PY = 'PYTHON'
>  try:
>      import ovs._json
> +    PARSER = PARSER_C
>  except ImportError:
> -    pass
> +    PARSER = PARSER_PY
>
>  __pychecker__ = 'no-stringiter'
>
> @@ -91,10 +94,9 @@ class Parser(object):
>      MAX_HEIGHT = 1000
>
>      def __new__(cls, *args, **kwargs):
> -        try:
> +        if PARSER == PARSER_C:
>              return ovs._json.Parser(*args, **kwargs)
> -        except NameError:
> -            return super(Parser, cls).__new__(cls)
> +        return super(Parser, cls).__new__(cls)
>
>      def __init__(self, check_trailer=False):
>          self.check_trailer = check_trailer
> diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
> index cc7b2cd..4a3027e 100644
> --- a/python/ovs/jsonrpc.py
> +++ b/python/ovs/jsonrpc.py
> @@ -272,7 +272,8 @@ class Connection(object):
>                  # data, so we convert it here as soon as possible.
>                  if data and not error:
>                      try:
> -                        data = decoder.decode(data)
> +                        if six.PY3 or ovs.json.PARSER == ovs.json.PARSER_PY:
> +                            data = decoder.decode(data)
>                      except UnicodeError:
>                          error = errno.EILSEQ
>                  if error:
> @@ -298,7 +299,11 @@ class Connection(object):
>              else:
>                  if self.parser is None:
>                      self.parser = ovs.json.Parser()
> -                self.input = self.input[self.parser.feed(self.input):]
> +                if six.PY3 and ovs.json.PARSER == ovs.json.PARSER_C:
> +                    self.input = self.input.encode('utf-8')[
> +                        self.parser.feed(self.input):].decode()
> +                else:
> +                    self.input = self.input[self.parser.feed(self.input):]
>                  if self.parser.is_done():
>                      msg = self.__process_msg()
>                      if msg:
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Acked-By: Lucas Alvares Gomes <lucasagomes@gmail.com>
Ben Pfaff Jan. 15, 2019, 7:17 p.m. UTC | #2
On Mon, Jan 14, 2019 at 08:15:36AM -0600, Terry Wilson wrote:
> This fix was reverted because it depended on a small bit of code
> in a patch that was reverted that changed some python/ovs testing
> and build. The fix is still necessary.
> 
> The OVS C-based JSON parser operates on bytes, so the parser_feed
> function returns the number of bytes that are processed. The pure
> Python JSON parser currently operates on unicode, so it expects
> that Parser.feed() returns a number of characters. This difference
> leads to parsing errors when unicode characters are passed to the
> C JSON parser from Python.
> 
> Signed-off-by: Terry Wilson <twilson@redhat.com>

Thanks, applied to master.  Let me know if this should be backported.
Daniel Alvarez Sanchez Jan. 16, 2019, 8:22 a.m. UTC | #3
Thanks for this!

On Tue, Jan 15, 2019 at 8:48 PM Ben Pfaff <blp@ovn.org> wrote:
>
> On Mon, Jan 14, 2019 at 08:15:36AM -0600, Terry Wilson wrote:
> > This fix was reverted because it depended on a small bit of code
> > in a patch that was reverted that changed some python/ovs testing
> > and build. The fix is still necessary.
> >
> > The OVS C-based JSON parser operates on bytes, so the parser_feed
> > function returns the number of bytes that are processed. The pure
> > Python JSON parser currently operates on unicode, so it expects
> > that Parser.feed() returns a number of characters. This difference
> > leads to parsing errors when unicode characters are passed to the
> > C JSON parser from Python.
> >
> > Signed-off-by: Terry Wilson <twilson@redhat.com>
>
> Thanks, applied to master.  Let me know if this should be backported.
It would be great to have it in 2.10 and 2.9 if possible :)
Thanks!
Daniel

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff Jan. 16, 2019, 4:01 p.m. UTC | #4
On Wed, Jan 16, 2019 at 09:22:07AM +0100, Daniel Alvarez Sanchez wrote:
> Thanks for this!
> 
> On Tue, Jan 15, 2019 at 8:48 PM Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Mon, Jan 14, 2019 at 08:15:36AM -0600, Terry Wilson wrote:
> > > This fix was reverted because it depended on a small bit of code
> > > in a patch that was reverted that changed some python/ovs testing
> > > and build. The fix is still necessary.
> > >
> > > The OVS C-based JSON parser operates on bytes, so the parser_feed
> > > function returns the number of bytes that are processed. The pure
> > > Python JSON parser currently operates on unicode, so it expects
> > > that Parser.feed() returns a number of characters. This difference
> > > leads to parsing errors when unicode characters are passed to the
> > > C JSON parser from Python.
> > >
> > > Signed-off-by: Terry Wilson <twilson@redhat.com>
> >
> > Thanks, applied to master.  Let me know if this should be backported.
> It would be great to have it in 2.10 and 2.9 if possible :)

OK, done.

Patch
diff mbox series

diff --git a/python/ovs/json.py b/python/ovs/json.py
index e84063f..f06204e 100644
--- a/python/ovs/json.py
+++ b/python/ovs/json.py
@@ -21,10 +21,13 @@  import sys
 
 import six
 
+PARSER_C = 'C'
+PARSER_PY = 'PYTHON'
 try:
     import ovs._json
+    PARSER = PARSER_C
 except ImportError:
-    pass
+    PARSER = PARSER_PY
 
 __pychecker__ = 'no-stringiter'
 
@@ -91,10 +94,9 @@  class Parser(object):
     MAX_HEIGHT = 1000
 
     def __new__(cls, *args, **kwargs):
-        try:
+        if PARSER == PARSER_C:
             return ovs._json.Parser(*args, **kwargs)
-        except NameError:
-            return super(Parser, cls).__new__(cls)
+        return super(Parser, cls).__new__(cls)
 
     def __init__(self, check_trailer=False):
         self.check_trailer = check_trailer
diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
index cc7b2cd..4a3027e 100644
--- a/python/ovs/jsonrpc.py
+++ b/python/ovs/jsonrpc.py
@@ -272,7 +272,8 @@  class Connection(object):
                 # data, so we convert it here as soon as possible.
                 if data and not error:
                     try:
-                        data = decoder.decode(data)
+                        if six.PY3 or ovs.json.PARSER == ovs.json.PARSER_PY:
+                            data = decoder.decode(data)
                     except UnicodeError:
                         error = errno.EILSEQ
                 if error:
@@ -298,7 +299,11 @@  class Connection(object):
             else:
                 if self.parser is None:
                     self.parser = ovs.json.Parser()
-                self.input = self.input[self.parser.feed(self.input):]
+                if six.PY3 and ovs.json.PARSER == ovs.json.PARSER_C:
+                    self.input = self.input.encode('utf-8')[
+                        self.parser.feed(self.input):].decode()
+                else:
+                    self.input = self.input[self.parser.feed(self.input):]
                 if self.parser.is_done():
                     msg = self.__process_msg()
                     if msg: