From 987265a6ef920466b44625097ff85f3e845dc8ea Mon Sep 17 00:00:00 2001
From: Sam Doran <sdoran@redhat.com>
Date: Fri, 20 Sep 2019 16:03:51 -0400
Subject: [PATCH] Account for empty strings when splitting the host pattern
 (#62442)

Improve tests
- add more unit test cases
- add specific integration test with more cases

Testing shows no major downside to calling .strip() twice in a comprehension vs. using a regular for loop and only calling .strip() once. Going with the comprehension for ease of maintenance and because comprehensions are optimized in CPython.
---
 .../split-host-pattern-empty-strings.yaml     |  2 ++
 lib/ansible/inventory/manager.py              |  2 +-
 .../targets/inventory_limit/aliases           |  1 +
 .../targets/inventory_limit/hosts.yml         |  5 +++
 .../targets/inventory_limit/runme.sh          | 31 +++++++++++++++++++
 .../units/plugins/inventory/test_inventory.py |  4 +++
 6 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/fragments/split-host-pattern-empty-strings.yaml
 create mode 100644 test/integration/targets/inventory_limit/aliases
 create mode 100644 test/integration/targets/inventory_limit/hosts.yml
 create mode 100755 test/integration/targets/inventory_limit/runme.sh

diff --git a/changelogs/fragments/split-host-pattern-empty-strings.yaml b/changelogs/fragments/split-host-pattern-empty-strings.yaml
new file mode 100644
index 00000000000..35af0f0e8bd
--- /dev/null
+++ b/changelogs/fragments/split-host-pattern-empty-strings.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+  - account for empty strings in when splitting the host pattern (https://github.com/ansible/ansible/issues/61964)
diff --git a/lib/ansible/inventory/manager.py b/lib/ansible/inventory/manager.py
index 14e61ba9334..72ffa6c8d70 100644
--- a/lib/ansible/inventory/manager.py
+++ b/lib/ansible/inventory/manager.py
@@ -130,7 +130,7 @@ def split_host_pattern(pattern):
                 '''), pattern, re.X
             )
 
-    return [p.strip() for p in patterns]
+    return [p.strip() for p in patterns if p.strip()]
 
 
 class InventoryManager(object):
diff --git a/test/integration/targets/inventory_limit/aliases b/test/integration/targets/inventory_limit/aliases
new file mode 100644
index 00000000000..3005e4b26d0
--- /dev/null
+++ b/test/integration/targets/inventory_limit/aliases
@@ -0,0 +1 @@
+shippable/posix/group4
diff --git a/test/integration/targets/inventory_limit/hosts.yml b/test/integration/targets/inventory_limit/hosts.yml
new file mode 100644
index 00000000000..2e1b1927303
--- /dev/null
+++ b/test/integration/targets/inventory_limit/hosts.yml
@@ -0,0 +1,5 @@
+all:
+  hosts:
+    host1:
+    host2:
+    host3:
diff --git a/test/integration/targets/inventory_limit/runme.sh b/test/integration/targets/inventory_limit/runme.sh
new file mode 100755
index 00000000000..6a142b3b1e2
--- /dev/null
+++ b/test/integration/targets/inventory_limit/runme.sh
@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+
+set -eux
+
+trap 'echo "Host pattern limit test failed"' ERR
+
+# https://github.com/ansible/ansible/issues/61964
+
+# These tests should return all hosts
+ansible -i hosts.yml all --limit ,, --list-hosts | tee out ; grep -q 'hosts (3)' out
+ansible -i hosts.yml ,, --list-hosts | tee out ; grep -q 'hosts (3)' out
+ansible -i hosts.yml , --list-hosts | tee out ; grep -q 'hosts (3)' out
+ansible -i hosts.yml all --limit , --list-hosts | tee out ; grep -q 'hosts (3)' out
+ansible -i hosts.yml all --limit '' --list-hosts | tee out ; grep -q 'hosts (3)' out
+
+
+# Only one host
+ansible -i hosts.yml all --limit ,,host1 --list-hosts | tee out ; grep -q 'hosts (1)' out
+ansible -i hosts.yml ,,host1 --list-hosts | tee out ; grep -q 'hosts (1)' out
+
+ansible -i hosts.yml all --limit host1,, --list-hosts | tee out ; grep -q 'hosts (1)' out
+ansible -i hosts.yml host1,, --list-hosts | tee out ; grep -q 'hosts (1)' out
+
+
+# Only two hosts
+ansible -i hosts.yml all --limit host1,,host3 --list-hosts | tee out ; grep -q 'hosts (2)' out
+ansible -i hosts.yml host1,,host3 --list-hosts | tee out ; grep -q 'hosts (2)' out
+
+ansible -i hosts.yml all --limit 'host1, ,    ,host3' --list-hosts | tee out ; grep -q 'hosts (2)' out
+ansible -i hosts.yml 'host1, ,    ,host3' --list-hosts | tee out ; grep -q 'hosts (2)' out
+
diff --git a/test/units/plugins/inventory/test_inventory.py b/test/units/plugins/inventory/test_inventory.py
index 48c3f9834a4..66b5ec37874 100644
--- a/test/units/plugins/inventory/test_inventory.py
+++ b/test/units/plugins/inventory/test_inventory.py
@@ -49,6 +49,10 @@ class TestInventory(unittest.TestCase):
         'a:b': ['a', 'b'],
         ' a : b ': ['a', 'b'],
         'foo:bar:baz[1:2]': ['foo', 'bar', 'baz[1:2]'],
+        'a,,b': ['a', 'b'],
+        'a,  ,b,,c, ,': ['a', 'b', 'c'],
+        ',': [],
+        '': [],
     }
 
     pattern_lists = [