On 03/15/2016 03:36 PM, Martin Babinsky wrote:
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.



Found some minor errors in the patch, attaching updated version.

--
Martin^3 Babinsky
From 33c251c4718ff0b92d885b89c80fc7d5a334651b 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 | 55 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 5d723b2c0ae7be07a5c89757b07ca353f23bf22e..1b3ba71c8039ef394e8fc087249f3e3a47c5ca56 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..b7e24a0a21c18a34dc8392d5ae0e8cd091bfa319 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,50 @@ 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)
+
+        print("\n")
+        qr.print_ascii(tty=stdout_is_tty)
+        print("\n")
 
         return rv
 
-- 
2.5.0

From 7e01302fddf9ddfc19f1e94785261e5fd76213a5 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 | 55 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index dbbc34ab14e0ed339bda93db379d70d95e7336ff..e76ff9833394429d133d6c55b29839133f2f27b8 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..9635b8f2faabeaa736a66205bacd45bc95bef9df 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,50 @@ 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)
+
+        print("\n")
+        qr.print_ascii(tty=stdout_is_tty)
+        print("\n")
 
         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