Operate pexpect with bytes to limit encoding issues (#73255)

* Operate pexpect with bytes to limit encoding issues

* Update tests to ensure no pepxect encoding issues

* Add changelog fragment

* Add multiline note

* Use rst formatting directly

changelogs/fragments/29351-expect-bytes.yml

@@ -0,0 +1,3 @@
+bugfixes:
+- expect - Operate pexpect with bytes to avoid potential encoding issues
+  (https://github.com/ansible/ansible/issues/29351)

lib/ansible/modules/expect.py

@@ -63,6 +63,9 @@ notes:
     C(/bin/bash -c "/path/to/something | grep else").
   - The question, or key, under I(responses) is a python regex match. Case
     insensitive searches are indicated with a prefix of C(?i).
+  - The C(pexpect) library used by this module operates with a search window
+    of 2000 bytes, and does not use a multiline regex match. To perform a
+    start of line bound match, use a pattern like ``(?m)^pattern``
   - By default, if a question is encountered multiple times, its string
     response will be repeated. If you need different responses for successive
     question matches, instead of a string response, use a list of strings as
@@ -108,11 +111,11 @@ except ImportError:
     HAS_PEXPECT = False
 
 from ansible.module_utils.basic import AnsibleModule, missing_required_lib
-from ansible.module_utils._text import to_native, to_text
+from ansible.module_utils._text import to_bytes, to_native, to_text
 
 
 def response_closure(module, question, responses):
-    resp_gen = (u'%s\n' % to_text(r).rstrip(u'\n') for r in responses)
+    resp_gen = (b'%s\n' % to_bytes(r).rstrip(b'\n') for r in responses)
 
     def wrapped(info):
         try:
@@ -156,9 +159,9 @@ def main():
         if isinstance(value, list):
             response = response_closure(module, key, value)
         else:
-            response = u'%s\n' % to_text(value).rstrip(u'\n')
+            response = b'%s\n' % to_bytes(value).rstrip(b'\n')
 
-        events[to_text(key)] = response
+        events[to_bytes(key)] = response
 
     if args.strip() == '':
         module.fail_json(rc=256, msg="no command given")
@@ -196,13 +199,18 @@ def main():
     try:
         try:
             # Prefer pexpect.run from pexpect>=4
-            out, rc = pexpect.run(args, timeout=timeout, withexitstatus=True,
+            b_out, rc = pexpect.run(args, timeout=timeout, withexitstatus=True,
                                     events=events, cwd=chdir, echo=echo,
-                                  encoding='utf-8')
+                                    encoding=None)
         except TypeError:
-            # Use pexpect.runu in pexpect>=3.3,<4
-            out, rc = pexpect.runu(args, timeout=timeout, withexitstatus=True,
-                                   events=events, cwd=chdir, echo=echo)
+            # Use pexpect._run in pexpect>=3.3,<4
+            # pexpect.run doesn't support `echo`
+            # pexpect.runu doesn't support encoding=None
+            b_out, rc = pexpect._run(args, timeout=timeout, withexitstatus=True,
+                                     events=events, extra_args=None, logfile=None,
+                                     cwd=chdir, env=None, _spawn=pexpect.spawn,
+                                     echo=echo)
+
     except (TypeError, AttributeError) as e:
         # This should catch all insufficient versions of pexpect
         # We deem them insufficient for their lack of ability to specify
@@ -217,12 +225,12 @@ def main():
     endd = datetime.datetime.now()
     delta = endd - startd
 
-    if out is None:
-        out = ''
+    if b_out is None:
+        b_out = b''
 
     result = dict(
         cmd=args,
-        stdout=out.rstrip('\r\n'),
+        stdout=to_native(b_out).rstrip('\r\n'),
         rc=rc,
         start=str(startd),
         end=str(endd),

test/integration/targets/expect/files/test_command.py

@@ -10,6 +10,16 @@ except NameError:
 
 prompts = sys.argv[1:] or ['foo']
 
+# latin1 encoded bytes
+# to ensure pexpect doesn't have any encoding errors
+data = b'premi\xe8re is first\npremie?re is slightly different\n????????? is Cyrillic\n? am Deseret\n'
+
+try:
+    sys.stdout.buffer.write(data)
+except AttributeError:
+    sys.stdout.write(data)
+print()
+
 for prompt in prompts:
     user_input = input_function(prompt)
     print(user_input)

test/integration/targets/expect/tasks/main.yml

@@ -43,7 +43,7 @@
   assert:
     that:
     - "expect_result.changed == true"
-    - "expect_result.stdout == 'foobar'"
+    - "expect_result.stdout_lines|last == 'foobar'"
 
 - name: test creates option
   expect:
@@ -71,7 +71,7 @@
   assert:
     that:
     - "creates_result.changed == true"
-    - "creates_result.stdout == 'foobar'"
+    - "creates_result.stdout_lines|last == 'foobar'"
 
 - name: test removes option
   expect:
@@ -85,7 +85,7 @@
   assert:
     that:
     - "removes_result.changed == true"
-    - "removes_result.stdout == 'foobar'"
+    - "removes_result.stdout_lines|last == 'foobar'"
 
 - name: test removes option (missing)
   expect:
@@ -139,9 +139,9 @@
 - name: assert echo works
   assert:
     that:
-    - "echo_result.stdout_lines|length == 2"
-    - "echo_result.stdout_lines[0] == 'foobar'"
-    - "echo_result.stdout_lines[1] == 'bar'"
+    - "echo_result.stdout_lines|length == 7"
+    - "echo_result.stdout_lines[-2] == 'foobar'"
+    - "echo_result.stdout_lines[-1] == 'bar'"
 
 - name: test response list
   expect:
@@ -155,9 +155,9 @@
 - name: assert list response works
   assert:
     that:
-    - "list_result.stdout_lines|length == 2"
-    - "list_result.stdout_lines[0] == 'foobar'"
-    - "list_result.stdout_lines[1] == 'foobaz'"
+    - "list_result.stdout_lines|length == 7"
+    - "list_result.stdout_lines[-2] == 'foobar'"
+    - "list_result.stdout_lines[-1] == 'foobaz'"
 
 - name: test no remaining responses
   expect: