Repository: RaRe-Technologies/smart_open (python)

Issue:
Reading a 0 B file from s3 raises KeyError
#### Problem description
A problem is somewhat similar to the one described here https://github.com/RaRe-Technologies/smart_open/issues/548 , though the Error is not the same. 

Be sure your description clearly answers the following questions:

- What are you trying to achieve?
I'm trying to read the file that might be empty in S3.
- What is the expected result?
The file is read without exceptions.
- What are you seeing instead?
KeyError exception is thrown

#### Steps/code to reproduce the problem
- Have an empty file in S3
- Run the following code
```python
from smart_open import open
with open('S3_uri', 'rb') as file:
    file.read()
```
Traceback:
```python
Traceback (most recent call last):
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 330, in _get
    return client.get_object(Bucket=bucket, Key=key, Range=range_string)
  File "/home/user/.local/lib/python3.8/site-packages/botocore/client.py", line 386, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/user/.local/lib/python3.8/site-packages/botocore/client.py", line 705, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidRange) when calling the GetObject operation: The requested range is not satisfiable

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 438, in _open_body
    response = _get(
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 338, in _get
    raise wrapped_error from error
OSError: unable to access bucket: 'mybucket' key: 'existing_file' version: None error: An error occurred (InvalidRange) when calling the GetObject operation: The requested range is not satisfiable

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/smart_open_lib.py", line 235, in open
    binary = _open_binary_stream(uri, binary_mode, transport_params)
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/smart_open_lib.py", line 398, in _open_binary_stream
    fobj = submodule.open_uri(uri, mode, transport_params)
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 224, in open_uri
    return open(parsed_uri['bucket_id'], parsed_uri['key_id'], mode, **kwargs)
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 291, in open
    fileobj = Reader(
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 574, in __init__
    self.seek(0)
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 666, in seek
    self._current_pos = self._raw_reader.seek(offset, whence)
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 417, in seek
    self._open_body(start, stop)
  File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 450, in _open_body
    self._position = self._content_length = int(error_response['ActualObjectSize'])
KeyError: 'ActualObjectSize'
```

#### Versions

Please provide the output of:

```python
smart_open 5.2.1
```

#### Checklist

Before you create the issue, please make sure you have:

- [X] Described the problem clearly
- [X] Provided a minimal reproducible example, including any required data
- [X] Provided the version numbers of the relevant software


Pull request description:
Fixes KeyError when retrieving empty but existing object from S3
#### Title

Attempt to fix the `KeyError` we have when reading an empty file from S3

#### Motivation

When we try to retrieve an empty file from S3 we may have the following error

```
...
File "/home/user/.local/lib/python3.8/site-packages/smart_open/s3.py", line 450, in _open_body
    self._position = self._content_length = int(error_response['ActualObjectSize'])
KeyError: 'ActualObjectSize'
```

This PR catches the `KeyError` and send a `get_object` query without any range to retrieve the data as suggested in the issue

- Fixes #667


#### Tests

I added a test, but it seems that the Moto lib provide the value `ActualObjectSize` even for empty objects so the code won't be tested...


Patch:
diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml
index c2cb5b3..767a691 100644
--- a/.github/workflows/python-package.yml
+++ b/.github/workflows/python-package.yml
@@ -6,10 +6,10 @@ jobs:
     steps:
       - uses: actions/checkout@v2
 
-      - name: Setup up Python 3.10
+      - name: Setup up Python 3.11
         uses: actions/setup-python@v2
         with:
-          python-version: "3.10"
+          python-version: "3.11"
 
       - name: Update pip
         run: python -m pip install -U pip
@@ -26,15 +26,15 @@ jobs:
     strategy:
       matrix:
         include:
-          - {python: '3.7', os: ubuntu-20.04}
           - {python: '3.8', os: ubuntu-20.04}
           - {python: '3.9', os: ubuntu-20.04}
           - {python: '3.10', os: ubuntu-20.04}
+          - {python: '3.11', os: ubuntu-20.04}
 
-          - {python: '3.7', os: windows-2019}
           - {python: '3.8', os: windows-2019}
           - {python: '3.9', os: windows-2019}
           - {python: '3.10', os: windows-2019}
+          - {python: '3.11', os: windows-2019}
     steps:
       - uses: actions/checkout@v2
 
@@ -63,10 +63,10 @@ jobs:
     strategy:
       matrix:
         include:
-          - {python: '3.7', os: ubuntu-20.04}
           - {python: '3.8', os: ubuntu-20.04}
           - {python: '3.9', os: ubuntu-20.04}
           - {python: '3.10', os: ubuntu-20.04}
+          - {python: '3.11', os: ubuntu-20.04}
 
           #
           # Some of the doctests don't pass on Windows because of Windows-specific
@@ -105,10 +105,10 @@ jobs:
     strategy:
       matrix:
         include:
-          - {python: '3.7', os: ubuntu-20.04, moto_server: true}
           - {python: '3.8', os: ubuntu-20.04}
           - {python: '3.9', os: ubuntu-20.04}
           - {python: '3.10', os: ubuntu-20.04}
+          - {python: '3.11', os: ubuntu-20.04}
 
           # Not sure why we exclude these, perhaps for historical reasons?
           #
@@ -159,10 +159,10 @@ jobs:
     strategy:
       matrix:
         include:
-          - {python: '3.7', os: ubuntu-20.04}
           - {python: '3.8', os: ubuntu-20.04}
           - {python: '3.9', os: ubuntu-20.04}
           - {python: '3.10', os: ubuntu-20.04}
+          - {python: '3.11', os: ubuntu-20.04}
 
           # - {python: '3.7', os: windows-2019}
           # - {python: '3.8', os: windows-2019}
diff --git a/setup.py b/setup.py
index 57ad9e6..8c08b8e 100644
--- a/setup.py
+++ b/setup.py
@@ -95,6 +95,7 @@ setup(
         'Programming Language :: Python :: 3.8',
         'Programming Language :: Python :: 3.9',
         'Programming Language :: Python :: 3.10',
+        'Programming Language :: Python :: 3.11',
         'Topic :: System :: Distributed Computing',
         'Topic :: Database :: Front-Ends',
     ],
diff --git a/smart_open/ftp.py b/smart_open/ftp.py
index 3dbe26f..7d4a5ad 100644
--- a/smart_open/ftp.py
+++ b/smart_open/ftp.py
@@ -14,6 +14,7 @@ import urllib.parse
 import smart_open.utils
 from ftplib import FTP, FTP_TLS, error_reply
 import types
+
 logger = logging.getLogger(__name__)
 
 SCHEMES = ("ftp", "ftps")
@@ -55,8 +56,13 @@ def open_uri(uri, mode, transport_params):
     uri_path = parsed_uri.pop("uri_path")
     scheme = parsed_uri.pop("scheme")
     secure_conn = True if scheme == "ftps" else False
-    return open(uri_path, mode, secure_connection=secure_conn,
-            transport_params=transport_params, **parsed_uri)
+    return open(
+        uri_path,
+        mode,
+        secure_connection=secure_conn,
+        transport_params=transport_params,
+        **parsed_uri,
+    )
 
 
 def convert_transport_params_to_args(transport_params):
@@ -90,7 +96,9 @@ def _connect(hostname, username, port, password, secure_connection, transport_pa
     try:
         ftp.login(username, password)
     except error_reply as e:
-        logger.error("Unable to login to FTP server: try checking the username and password!")
+        logger.error(
+            "Unable to login to FTP server: try checking the username and password!"
+        )
         raise e
     if secure_connection:
         ftp.prot_p()
@@ -99,7 +107,7 @@ def _connect(hostname, username, port, password, secure_connection, transport_pa
 
 def open(
     path,
-    mode="r",
+    mode="rb",
     host=None,
     user=None,
     password=None,
@@ -146,6 +154,7 @@ def open(
     except KeyError:
         raise ValueError(f"unsupported mode: {mode!r}")
     ftp_mode, file_obj_mode = mode_to_ftp_cmds[mode]
+    conn.voidcmd("TYPE I")
     socket = conn.transfercmd(f"{ftp_mode} {path}")
     fobj = socket.makefile(file_obj_mode)
 
@@ -153,6 +162,7 @@ def open(
         self.orig_close()
         self.socket.close()
         self.conn.close()
+
     fobj.orig_close = fobj.close
     fobj.socket = socket
     fobj.conn = conn
diff --git a/smart_open/s3.py b/smart_open/s3.py
index b211163..7e3fb63 100644
--- a/smart_open/s3.py
+++ b/smart_open/s3.py
@@ -324,10 +324,13 @@ def open(
 
 def _get(client, bucket, key, version, range_string):
     try:
+        params = dict(Bucket=bucket, Key=key)
         if version:
-            return client.get_object(Bucket=bucket, Key=key, VersionId=version, Range=range_string)
-        else:
-            return client.get_object(Bucket=bucket, Key=key, Range=range_string)
+            params["VersionId"] = version
+        if range_string:
+            params["Range"] = range_string
+
+        return client.get_object(**params)
     except botocore.client.ClientError as error:
         wrapped_error = IOError(
             'unable to access bucket: %r key: %r version: %r error: %s' % (
@@ -447,8 +450,19 @@ class _SeekableRawReader(object):
             error_response = _unwrap_ioerror(ioe)
             if error_response is None or error_response.get('Code') != _OUT_OF_RANGE:
                 raise
-            self._position = self._content_length = int(error_response['ActualObjectSize'])
-            self._body = io.BytesIO()
+            try:
+                self._position = self._content_length = int(error_response['ActualObjectSize'])
+                self._body = io.BytesIO()
+            except KeyError:
+                response = _get(
+                    self._client,
+                    self._bucket,
+                    self._key,
+                    self._version_id,
+                    None,
+                )
+                self._position = self._content_length = response["ContentLength"]
+                self._body = response["Body"]
         else:
             #
             # Keep track of how many times boto3's built-in retry mechanism
@@ -461,7 +475,7 @@ class _SeekableRawReader(object):
                 self,
                 response['ResponseMetadata']['RetryAttempts'],
             )
-            units, start, stop, length = smart_open.utils.parse_content_range(response['ContentRange'])
+            _, start, stop, length = smart_open.utils.parse_content_range(response['ContentRange'])
             self._content_length = length
             self._position = start
             self._body = response['Body']


Test patch:
diff --git a/smart_open/tests/test_s3.py b/smart_open/tests/test_s3.py
index a91a731..fa90710 100644
--- a/smart_open/tests/test_s3.py
+++ b/smart_open/tests/test_s3.py
@@ -73,6 +73,8 @@ def patch_invalid_range_response(actual_size):
                 error_response['ActualObjectSize'] = actual_size
                 error_response['Code'] = 'InvalidRange'
                 error_response['Message'] = 'The requested range is not satisfiable'
+            if actual_size is None:
+                error_response.pop('ActualObjectSize', None)
             raise
 
     with mock.patch('smart_open.s3._get', new=mock_get):
@@ -399,6 +401,15 @@ class ReaderTest(BaseTest):
 
         self.assertEqual(data, b'')
 
+    def test_read_empty_file_no_actual_size(self):
+        _resource('s3').Object(BUCKET_NAME, KEY_NAME).put(Body=b'')
+
+        with self.assertApiCalls(GetObject=2), patch_invalid_range_response(None):
+            with smart_open.s3.Reader(BUCKET_NAME, KEY_NAME) as fin:
+                data = fin.read()
+
+        self.assertEqual(data, b'')
+
 
 @moto.mock_s3
 class MultipartWriterTest(unittest.TestCase):