Repository: pandas-dev/pandas
Pull request: ENH: loosen XLS signature
- [x] closes #41225
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

Added ability to check for multiple XLS signatures according to testing files available at [Spreadsheet Project](https://www.openoffice.org/sc/testdocs/). Also defer raising an error when the engine is specified but the file signature does not match one of the values in `SIGNATURES`: this allows a user to attempt to specify and use an engine even if the passed file doesn't match one of the provided values in `SIGNATURES`.

This is my first PR for this project, so please let me know if more is expected (writing tests, writing a whatsnew entry, etc). I have run Flake8 on the code and successfully opened BIFF2 through BIFF8 files with this method. Thanks!


Problem statement:
ENH: The XLS_SIGNATURE is too restrictive
Pandas 1.2.4 fails to open XLS files generated by Lotus 1-2-3 because they have a different header than the one expected in `XLS_SIGNATURE` (`io.excel._base` line 1001 and then 1050).

I know I'm likely to be the only person in the world with this issue, but my boss still uses Lotus 1-2-3 🙄 

I'm willing to submit a pull request with my fix if people are okay with it. The fix basically involves changing `XLS_SIGNATURE` to a list:

```
XLS_SIGNATURES = [
  b"\xD0\xCF\x11\xE0\xA1\xB1\xA1\xE1",
  b"\t\x04\x06",
]
ZIP_SIGNATURE = b"PK\x03\x04"
PEEK_SIZE = max(map(len, XLS_SIGNATURES + [ZIP_SIGNATURE]))
```

and then later:

```
if any(peek.startswith(signature) for signature in XLS_SIGNATURES):
  return "xls"
```

I don't know if the second `XLS_SIGNATURES` value is "good," but it works. I can attach an XLS that was exported by 1-2-3 if it would be beneficial to others, or if they'd be able to help me find a better second value for `XLS_SIGNATURES`. I tried looking through the byte-code for anything similar to the "DOCFILE" XLS signature, but didn't see anything.

Hints from issue discussion:
Thanks for the report! An example sheet would be helpful to further diagnose.
@rhshadrach See attached. If you have any suggestions I'd be happy to put together a PR.

[PANDAS.XLS](https://github.com/pandas-dev/pandas/files/6415725/PANDAS.XLS) 

For future reference: OpenOffice [has made available](https://www.openoffice.org/sc/testdocs/) the various BIFF files for testing.
Thanks @geoffrey-eisenbarth - documentation is hard to find, but I think this is a BIFF4 format:

http://www.nationalarchives.gov.uk/PRONOM/Format/proFormatSearch.aspx?status=detailReport&id=681&strPageToDisplay=signatures

This agrees with your code: `b"\x09\x04\x06\x00\x00\x00"` (note I'm replacing your usage of `\t`). What you suggested would be a welcome PR.

However, it seems likely to me there are other file signatures that will still go unrecognized. One of the shortfalls of the current pandas implementation is that if the user specifies ``engine`` but we can't verify the file is correct, we will raise without just trying the engine.

Instead, I think `inspect_excel_format` should return None if a format cannot be determined instead of raising. Then, we should raise if engine is None with a message to the effect "Excel file format cannot be determined, you must specify an engine manually", and otherwise attempt to use the engine the user specified.

Would you be interested in putting up a PR for this @geoffrey-eisenbarth?
@rhshadrach I agree with your suggested route, and I'd love to submit the PR. Hoping to have time tomorrow to dig more into the code and reference your suggestion with `inspect_excel_format`. Should be a fast PR. 

Thanks for your help!

Commit message: Add relevant whatsnew entry.

Patch to review:
diff --git a/doc/source/whatsnew/v1.2.5.rst b/doc/source/whatsnew/v1.2.5.rst
index 60e146b2212eb..09c7239c5c3ec 100644
--- a/doc/source/whatsnew/v1.2.5.rst
+++ b/doc/source/whatsnew/v1.2.5.rst
@@ -34,8 +34,7 @@ Bug fixes
 
 Other
 ~~~~~
-
--
+- Loosen XLS signatures used in :func:`read_excel` to determine if the `xlrd` engine should be used (:issue:`41225`)
 -
 
 .. ---------------------------------------------------------------------------
diff --git a/pandas/io/excel/_base.py b/pandas/io/excel/_base.py
index d26a991ba2820..ea6f27a057780 100644
--- a/pandas/io/excel/_base.py
+++ b/pandas/io/excel/_base.py
@@ -10,6 +10,7 @@
 from typing import (
     Any,
     Mapping,
+    Optional,
     cast,
 )
 import warnings
@@ -1014,16 +1015,22 @@ def close(self):
         return content
 
 
-XLS_SIGNATURE = b"\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1"
-ZIP_SIGNATURE = b"PK\x03\x04"
-PEEK_SIZE = max(len(XLS_SIGNATURE), len(ZIP_SIGNATURE))
+SIGNATURES = {
+    "biff2": b"\x09\x00\x04\x00",
+    "biff3": b"\x09\x02\x06\x00",
+    "biff4": b"\x09\x04\x06\x00",
+    "biff5": b"\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1",
+    "biff8": b"\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1",
+    "zip": b"PK\x03\x04",
+}
+PEEK_SIZE = max(map(len, SIGNATURES.values()))
 
 
 @doc(storage_options=_shared_docs["storage_options"])
 def inspect_excel_format(
     content_or_path: FilePathOrBuffer,
     storage_options: StorageOptions = None,
-) -> str:
+) -> Optional[str]:
     """
     Inspect the path or content of an excel file and get its format.
 
@@ -1037,8 +1044,8 @@ def inspect_excel_format(
 
     Returns
     -------
-    str
-        Format of file.
+    str or None
+        Format of file (if it can be determined)
 
     Raises
     ------
@@ -1063,10 +1070,14 @@ def inspect_excel_format(
             peek = buf
         stream.seek(0)
 
-        if peek.startswith(XLS_SIGNATURE):
+        if any(
+            peek.startswith(signature)
+            for (file_format, signature) in SIGNATURES.items()
+            if file_format.startswith("biff")
+        ):
             return "xls"
-        elif not peek.startswith(ZIP_SIGNATURE):
-            raise ValueError("File is not a recognized excel file")
+        elif not peek.startswith(SIGNATURES["zip"]):
+            return None
 
         # ZipFile typing is overly-strict
         # https://github.com/python/typeshed/issues/4212
@@ -1174,8 +1185,12 @@ def __init__(
                 ext = inspect_excel_format(
                     content_or_path=path_or_buffer, storage_options=storage_options
                 )
+                if ext is None:
+                    raise ValueError(
+                        "Excel file format cannot be determined, you must specify "
+                        "an engine manually."
+                    )
 
-            # ext will always be valid, otherwise inspect_excel_format would raise
             engine = config.get_option(f"io.excel.{ext}.reader", silent=True)
             if engine == "auto":
                 engine = get_default_engine(ext, mode="reader")
@@ -1190,12 +1205,13 @@ def __init__(
                         path_or_buffer, storage_options=storage_options
                     )
 
-            if ext != "xls" and xlrd_version >= "2":
+            # Pass through if ext is None, otherwise check if ext valid for xlrd
+            if ext and ext != "xls" and xlrd_version >= "2":
                 raise ValueError(
                     f"Your version of xlrd is {xlrd_version}. In xlrd >= 2.0, "
                     f"only the xls format is supported. Install openpyxl instead."
                 )
-            elif ext != "xls":
+            elif ext and ext != "xls":
                 caller = inspect.stack()[1]
                 if (
                     caller.filename.endswith(
diff --git a/pandas/tests/io/excel/test_readers.py b/pandas/tests/io/excel/test_readers.py
index c4b3221e1d3a7..69f5436a7476a 100644
--- a/pandas/tests/io/excel/test_readers.py
+++ b/pandas/tests/io/excel/test_readers.py
@@ -9,6 +9,7 @@
 
 import numpy as np
 import pytest
+from xlrd import XLRDError
 
 import pandas.util._test_decorators as td
 
@@ -718,9 +719,18 @@ def test_missing_file_raises(self, read_ext):
 
     def test_corrupt_bytes_raises(self, read_ext, engine):
         bad_stream = b"foo"
-        if engine is None or engine == "xlrd":
+        if engine is None:
             error = ValueError
-            msg = "File is not a recognized excel file"
+            msg = (
+                "Excel file format cannot be determined, you must "
+                "specify an engine manually."
+            )
+        elif engine == "xlrd":
+            error = XLRDError
+            msg = (
+                "Unsupported format, or corrupt file: Expected BOF "
+                "record; found b'foo'"
+            )
         else:
             error = BadZipFile
             msg = "File is not a zip file"


Code review comments:
diff_hunk: @@ -1014,16 +1015,22 @@ def close(self):
         return content
 
 
-XLS_SIGNATURE = b"\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1"
-ZIP_SIGNATURE = b"PK\x03\x04"
-PEEK_SIZE = max(len(XLS_SIGNATURE), len(ZIP_SIGNATURE))
+SIGNATURES = {
+    "biff2": b"\x09\x00\x04\x00",
+    "biff3": b"\x09\x02\x06\x00",
+    "biff4": b"\x09\x04\x06\x00",
+    "biff5": b"\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1",
+    "biff8": b"\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1",
+    "zip": b"PK\x03\x04",
+}
+PEEK_SIZE = max(map(len, SIGNATURES.values()))
 
 
 @doc(storage_options=_shared_docs["storage_options"])
 def inspect_excel_format(
     content_or_path: FilePathOrBuffer,
     storage_options: StorageOptions = None,
-) -> str:
+) -> Optional[str]:
path: pandas/io/excel/_base.py
text: @user1:
str | None instead of optional

diff_hunk: @@ -34,8 +34,7 @@ Bug fixes
 
 Other
 ~~~~~
-
--
+- Loosen XLS signatures used in :func:`read_excel` to determine if the `xlrd` engine should be used (:issue:`41225`)
path: doc/source/whatsnew/v1.2.5.rst
text: @user1:
Please move to 1.3, I think we have a section for IO excel or something like that