On 03/09/2016 07:06 AM, Jan Cholasta wrote:
On 8.3.2016 17:45, Martin Babinsky wrote:
On 03/08/2016 05:35 PM, Jan Cholasta wrote:
Hi,

On 8.3.2016 16:21, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/5700

1) Instead of checking for utf-8 in particular, I would prefer a more
robust approach:

try:
     qr = qrcode.QRCode()
     qr.add_data('test')
     qr.make()
     qr.print_ascii(tty=True)
except UnicodeError:
     # it is not printable
else:
     # it is printable

Now you mean the check in the _check_qrcode_capability() or the
_print_qrcode() method itself?

_check_qrcode_capability() of course.


2) There is no os.isatty() check to see if stdout is actually a tty.

This check is performed inside both print_ascii() and print_tty()
methods of QRCode object, but you probably mean that I should put the
check also into _check_qrcode_capability() method, right?

Yes. If stdout is not a tty, we should at least not tty=True in
print_ascii().


Honza





Attaching updated patch. After the discussion with other developers we decided to just print warnings when non-UTF-8 encoding is used and tty width is smaller that the QR code size.

--
Martin^3 Babinsky
From 7dbb9ce800b9db1508130a9e95ed3817e576f276 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Tue, 8 Mar 2016 15:56:52 +0100
Subject: [PATCH] otptoken-add: improve the robustness of QR code printing

The python-qrcode print_ascii() method does not work in terminals with
non-UTF-8 encoding. When this is the case do not render QR code but print a
warning instead. Also print a warning when the QR code size is greater that
terminal width if the output is a tty.

https://fedorahosted.org/freeipa/ticket/5700
---
 ipalib/messages.py         | 21 ++++++++++++++++++
 ipalib/plugins/otptoken.py | 53 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 5d723b2c0ae7be07a5c89757b07ca353f23bf22e..88e0ce9df6a7c4c32642cfcb6e6710dbb33bdbb6 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -342,6 +342,27 @@ class BrokenTrust(PublicMessage):
                "running 'ipa trust-add' again.")
 
 
+class QRCodeRenderingSkipped(PublicMessage):
+    """
+    **13019** QR code rendering was skipped because the output does not support
+    it
+    """
+    errno = 13019
+    type = "warning"
+    format = _("Could not render QR code: Non-UTF 8 output encoding detected")
+
+
+class QRCodeTooWide(PublicMessage):
+    """
+    **13020** QR code width is larger than that of output TTY
+    it
+    """
+    errno = 13020
+    type = "warning"
+    format = _("QR code width is larger than that of output TTY. "
+               "Please increase your terminal size")
+
+
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
     """
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 846155dfb3af53ad65f3a4f488629837d10a2bce..f242f9c3c2633f55429b758a39867c60c96ef6cb 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -18,23 +18,28 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import print_function
+import sys
 
 from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMember
 from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
 from ipalib import api, Int, Str, Bool, DateTime, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
+from ipalib import messages
 from ipalib.plugable import Registry
 from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound, ValidationError
 from ipalib.request import context
 from ipalib.frontend import Local
 from ipaplatform.paths import paths
 from ipapython.nsslib import NSSConnection
+from ipapython.version import API_VERSION
 
 import base64
+import locale
 import uuid
 import qrcode
 import os
 
 import six
+from six import StringIO
 from six.moves import urllib
 
 if six.PY3:
@@ -352,16 +357,48 @@ class otptoken_add(LDAPCreate):
 
     def output_for_cli(self, textui, output, *args, **options):
         uri = output['result'].get('uri', None)
-        rv = super(otptoken_add, self).output_for_cli(textui, output, *args, **options)
+
+        if uri is None or options.get('no_qrcode', False):
+            return super(otptoken_add, self).output_for_cli(
+                textui, output, *args, **options)
 
         # Print QR code to terminal if specified
-        if uri and not options.get('no_qrcode', False):
-            print("\n")
-            qr = qrcode.QRCode()
-            qr.add_data(uri)
-            qr.make()
-            qr.print_ascii(tty=True)
-            print("\n")
+        qr_output = StringIO()
+        qr = qrcode.QRCode()
+        qr.add_data(uri)
+        qr.make()
+        qr.print_ascii(out=qr_output, tty=False)
+
+        encoding = getattr(sys.stdout, 'encoding', None)
+        if encoding is None:
+            encoding = locale.getpreferredencoding(False)
+
+        try:
+            qr_code = qr_output.getvalue().decode(encoding)
+        except UnicodeError:
+            messages.add_message(
+                API_VERSION,
+                output,
+                message=messages.QRCodeRenderingSkipped()
+            )
+            return super(otptoken_add, self).output_for_cli(
+                textui, output, *args, **options)
+
+        stdout_is_tty = sys.stdout.isatty()
+        if stdout_is_tty:
+            output_width = self.api.Backend.textui.get_tty_width()
+            qr_code_width = len(qr_code.split('\n')[0])
+            if qr_code_width > output_width:
+                messages.add_message(
+                    API_VERSION,
+                    output,
+                    message=messages.QRCodeTooWide()
+                )
+
+        rv = super(otptoken_add, self).output_for_cli(
+                textui, output, *args, **options)
+
+        qr.print_ascii(tty=stdout_is_tty)
 
         return rv
 
-- 
2.5.0

From 997f09454f8b206d9e5c9cee0d12a71df042161d Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Tue, 8 Mar 2016 15:56:52 +0100
Subject: [PATCH] otptoken-add: improve the robustness of QR code printing

The python-qrcode print_ascii() method does not work in terminals with
non-UTF-8 encoding. When this is the case do not render QR code but print a
warning instead. Also print a warning when the QR code size is greater that
terminal width if the output is a tty.

https://fedorahosted.org/freeipa/ticket/5700
---
 ipalib/messages.py         | 21 ++++++++++++++++++
 ipalib/plugins/otptoken.py | 53 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index dbbc34ab14e0ed339bda93db379d70d95e7336ff..da8abcc589805e70afd09f634e9a69037cf3c024 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -352,6 +352,27 @@ class BrokenTrust(PublicMessage):
                "running 'ipa trust-add' again.")
 
 
+class QRCodeRenderingSkipped(PublicMessage):
+    """
+    **13019** QR code rendering was skipped because the output does not support
+    it
+    """
+    errno = 13019
+    type = "warning"
+    format = _("Could not render QR code: Non-UTF 8 output encoding detected")
+
+
+class QRCodeTooWide(PublicMessage):
+    """
+    **13020** QR code width is larger than that of output TTY
+    it
+    """
+    errno = 13020
+    type = "warning"
+    format = _("QR code width is larger than that of output TTY. "
+               "Please increase your terminal size")
+
+
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
     """
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 2d7e99d06f4f64319bbd2f8f13a60e5391a57d77..e3cbc2363639c2e0ada5bbc147f7843150ab5c47 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -18,10 +18,12 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import print_function
+import sys
 
 from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMember
 from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
 from ipalib import api, Int, Str, Bool, DateTime, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
+from ipalib import messages
 from ipalib.plugable import Registry
 from ipalib.errors import (
     PasswordMismatch,
@@ -32,13 +34,16 @@ from ipalib.request import context
 from ipalib.frontend import Local
 from ipaplatform.paths import paths
 from ipapython.nsslib import NSSConnection
+from ipapython.version import API_VERSION
 
 import base64
+import locale
 import uuid
 import qrcode
 import os
 
 import six
+from six import StringIO
 from six.moves import urllib
 
 if six.PY3:
@@ -356,16 +361,48 @@ class otptoken_add(LDAPCreate):
 
     def output_for_cli(self, textui, output, *args, **options):
         uri = output['result'].get('uri', None)
-        rv = super(otptoken_add, self).output_for_cli(textui, output, *args, **options)
+
+        if uri is None or options.get('no_qrcode', False):
+            return super(otptoken_add, self).output_for_cli(
+                textui, output, *args, **options)
 
         # Print QR code to terminal if specified
-        if uri and not options.get('no_qrcode', False):
-            print("\n")
-            qr = qrcode.QRCode()
-            qr.add_data(uri)
-            qr.make()
-            qr.print_ascii(tty=True)
-            print("\n")
+        qr_output = StringIO()
+        qr = qrcode.QRCode()
+        qr.add_data(uri)
+        qr.make()
+        qr.print_ascii(out=qr_output, tty=False)
+
+        encoding = getattr(sys.stdout, 'encoding', None)
+        if encoding is None:
+            encoding = locale.getpreferredencoding(False)
+
+        try:
+            qr_code = qr_output.getvalue().decode(encoding)
+        except UnicodeError:
+            messages.add_message(
+                API_VERSION,
+                output,
+                message=messages.QRCodeRenderingSkipped()
+            )
+            return super(otptoken_add, self).output_for_cli(
+                textui, output, *args, **options)
+
+        stdout_is_tty = sys.stdout.isatty()
+        if stdout_is_tty:
+            output_width = self.api.Backend.textui.get_tty_width()
+            qr_code_width = len(qr_code.split('\n')[0])
+            if qr_code_width > output_width:
+                messages.add_message(
+                    API_VERSION,
+                    output,
+                    message=messages.QRCodeTooWide()
+                )
+
+        rv = super(otptoken_add, self).output_for_cli(
+                textui, output, *args, **options)
+
+        qr.print_ascii(tty=stdout_is_tty)
 
         return rv
 
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to