From 8288b5bb76d01c7fb88c51672bfb5d33e077d2d8 Mon Sep 17 00:00:00 2001
From: Jan Jansky <jjansky@redhat.com>
Date: Thu, 8 Oct 2020 14:26:03 +0200
Subject: [PATCH] [policy] Fix failure conditions with upload

The logic for determining if an archive should be uploaded to the
Customer Portal was too strict, ease it to now properly only block on a
missing case number since username and passwords may now be provided via
env vars.

Fixes an issue whereby we ignore a user-provided FTP directory.

Adds a timeout and a timeout handler for FTP connections, rather than
letting the connection attempt continue indefinitely.

Second, adds exception handling for an edge case where the connection to
the FTP server fails, but does not generate an exception from the ftplib
module.

Additionally, correct the type-ing of the error numbers being checked so
that we actually match them.

Caling "sos report --upload --case-id=123 --batch" should fallback
to uploading to FTP server as the upload user is unknown and can't
be prompted in batch mode.

Related: #2276
Related: #2245
Resolves: #2265

Signed-off-by: Jan Jansky <jjansky@redhat.com>
Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>
---
 sos/policies/__init__.py | 19 +++++++++++++------
 sos/policies/redhat.py   | 21 +++++++++++++++++----
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/sos/policies/__init__.py b/sos/policies/__init__.py
index ed3f0cc..a22c277 100644
--- a/sos/policies/__init__.py
+++ b/sos/policies/__init__.py
@@ -972,7 +972,8 @@ class LinuxPolicy(Policy):
         """Should be overridden by policies to determine if a user needs to
         be provided or not
         """
-        if not self.upload_user and not self._upload_user:
+        if not self.get_upload_password() and (self.get_upload_user() !=
+                                               self._upload_user):
             msg = "Please provide upload user for %s: " % self.get_upload_url()
             self.upload_user = input(_(msg))
 
@@ -1029,7 +1030,8 @@ class LinuxPolicy(Policy):
 
         """
         self.upload_archive = archive
-        self.upload_url = self.get_upload_url()
+        if not self.upload_url:
+            self.upload_url = self.get_upload_url()
         if not self.upload_url:
             raise Exception("No upload destination provided by policy or by "
                             "--upload-url")
@@ -1187,18 +1189,23 @@ class LinuxPolicy(Policy):
             password = self.get_upload_password()
 
         if not directory:
-            directory = self._upload_directory
+            directory = self.upload_directory or self._upload_directory
 
         try:
-            session = ftplib.FTP(url, user, password)
+            session = ftplib.FTP(url, user, password, timeout=15)
+            if not session:
+                raise Exception("connection failed, did you set a user and "
+                                "password?")
             session.cwd(directory)
+        except socket.timeout:
+            raise Exception("timeout hit while connecting to %s" % url)
         except socket.gaierror:
             raise Exception("unable to connect to %s" % url)
         except ftplib.error_perm as err:
             errno = str(err).split()[0]
-            if errno == 503:
+            if errno == '503':
                 raise Exception("could not login as '%s'" % user)
-            if errno == 550:
+            if errno == '550':
                 raise Exception("could not set upload directory to %s"
                                 % directory)
 
diff --git a/sos/policies/redhat.py b/sos/policies/redhat.py
index 9fbe743..3412f44 100644
--- a/sos/policies/redhat.py
+++ b/sos/policies/redhat.py
@@ -312,15 +312,28 @@ support representative.
                 "Enter your Red Hat Customer Portal username (empty to use "
                 "public dropbox): ")
             )
+            if not self.upload_user:
+                self.upload_url = RH_FTP_HOST
+                self.upload_user = self._upload_user
+
+    def _upload_user_set(self):
+        user = self.get_upload_user()
+        return user and (user != 'anonymous')
 
     def get_upload_url(self):
+        if self.upload_url:
+            return self.upload_url
         if self.commons['cmdlineopts'].upload_url:
             return self.commons['cmdlineopts'].upload_url
-        if (not self.case_id or not self.upload_user or not
-                self.upload_password):
-            # Cannot use the RHCP. Use anonymous dropbox
+        # anonymous FTP server should be used as fallback when either:
+        # - case id is not set, or
+        # - upload user isn't set AND batch mode prevents to prompt for it
+        if (not self.case_id) or \
+           ((not self._upload_user_set()) and
+               self.commons['cmdlineopts'].batch):
             self.upload_user = self._upload_user
-            self.upload_directory = self._upload_directory
+            if self.upload_directory is None:
+                self.upload_directory = self._upload_directory
             self.upload_password = None
             return RH_FTP_HOST
         else:
-- 
1.8.3.1