[ENG-8488] MFR: .xlsx file not rendering correctly in OSF Storage (CenterForOpenScience#396)

Author: opaduchak

mfr/core/exceptions.py

@@ -146,6 +146,13 @@ def __init__(self, message, *args, metadata_url: str = '', response: str = '', *
             'response': self.response
         }])
 
+class CorruptedError(RendererError):
+
+    __TYPE = 'corrupted'
+
+    def __init__(self, *args, renderer_class: str = '', **kwargs):
+        super().__init__("File is corrupted, impossible to render, please check it's integrity", *args, renderer_class, **kwargs)
+
 class TooBigToRenderError(ProviderError):
     """If the user tries to render a file larger than a server specified maximum, throw a
     TooBigToRenderError.

mfr/extensions/tabular/libs/__init__.py

@@ -31,10 +31,14 @@ def sav_pandas():
     from ..libs.panda_tools import sav_pandas
     return sav_pandas
 
+def xls():
+    from ..libs.xlrd_tools import xls
+    return xls
 
-def xlsx_xlrd():
-    from ..libs.xlrd_tools import xlsx_xlrd
-    return xlsx_xlrd
+
+def xlsx():
+    from ..libs.xlrd_tools import xlsx
+    return xlsx
 
 def mat_h5py_scipy():
     from ..libs.h5py_scipy_tools import mat_h5py_scipy

mfr/extensions/tabular/libs/xlrd_tools.py

@@ -11,7 +11,7 @@
 )
 
 
-def xlsx_xlrd(fp):
+def xls(fp):
     """
     • .xls → xlrd
     • .xlsx → openpyxl (xlrd ≥2.0 dropped xlsx support)
@@ -21,13 +21,11 @@ def xlsx_xlrd(fp):
     ZipFile) can seek inside safely.
     """
     sheets = OrderedDict()
+    wb = xlrd.open_workbook(file_contents=to_bytes(fp))
+    return parse_xls(wb, sheets)
 
-    try:
-        wb = xlrd.open_workbook(file_contents=to_bytes(fp))
-        return parse_xls(wb, sheets)
-    except xlrd.biffh.XLRDError:
-        pass
-
+def xlsx(fp):
+    sheets = OrderedDict()
     try:
         wb = load_workbook(BytesIO(to_bytes(fp)), data_only=True, read_only=True)
     except zipfile.BadZipFile as exc:

mfr/extensions/tabular/settings.py

@@ -12,9 +12,9 @@
 LIBS = config.get_object('LIBS', {
     '.csv': [libs.csv_stdlib],
     '.tsv': [libs.csv_stdlib],
-    '.gsheet': [libs.xlsx_xlrd],
-    '.xlsx': [libs.xlsx_xlrd],
-    '.xls': [libs.xlsx_xlrd],
+    '.gsheet': [libs.xlsx],
+    '.xlsx': [libs.xlsx],
+    '.xls': [libs.xls],
     '.dta': [libs.dta_pandas],
     '.sav': [libs.sav_stdlib],
     '.mat': [libs.mat_h5py_scipy],

mfr/extensions/tabular/utilities.py

@@ -8,7 +8,7 @@
 from tempfile import NamedTemporaryFile
 
 from mfr.extensions.tabular import compat
-from mfr.core.exceptions import SubprocessError, TooBigToRenderError
+from mfr.core.exceptions import SubprocessError, TooBigToRenderError, CorruptedError
 from mfr.extensions.tabular.settings import (PSPP_CONVERT_BIN,
                                              PSPP_CONVERT_TIMEOUT)
 
@@ -143,28 +143,22 @@ def parse_xls(wb, sheets):
 def parse_xlsx(wb, sheets):
     for name in wb.sheetnames:
         ws = wb[name]
-        max_row = ws.max_row or 0
-        max_col = ws.max_column or 0
-        verify_size(max_row, max_col, '.xlsx')
-
-        if max_row == 0 or max_col == 0:
-            sheets[name] = ([], [])
-            continue
-
         header_row = next(ws.iter_rows(max_row=1, values_only=True), [])
         fields = fix_headers(header_row)
         rows = [
             dict(zip(fields, row))
             for row in ws.iter_rows(min_row=2,
-                                    max_row=max_row,
-                                    max_col=max_col,
+                                    max_row=MAX_SIZE,
+                                    max_col=MAX_SIZE,
                                     values_only=True)
         ]
         sheets[name] = (header_population(fields), rows)
     return sheets
 
 
 def verify_size(rows, cols, ext):
+    if rows is None or cols is None:
+        raise CorruptedError
     if rows > MAX_SIZE or cols > MAX_SIZE:
         raise TooBigToRenderError('Table is too large to render.', ext,
                                nbr_cols=cols, nbr_rows=rows)

tests/extensions/tabular/test_xlsx_tools.py

@@ -9,7 +9,7 @@ class TestTabularPandaTools:
 
     def test_xlsx_xlrd(self):
         with open(os.path.join(BASE, 'files', 'test.xlsx'), 'rb') as fp:
-            sheets = xlrd_tools.xlsx_xlrd(fp)
+            sheets = xlrd_tools.xlsx(fp)
 
         sheet = sheets.popitem()[1]
         assert sheet[0][0] == {'field': 'one', 'id': 'one', 'name': 'one', 'sortable': True}