[ENG-8489] Enforce MAX_SIZE on .xls rendering just like was on .xlsx (#397)

Author: sh-andriy

mfr/extensions/tabular/libs/xlrd_tools.py

@@ -13,18 +13,16 @@
 
 def xls(fp):
     """
-    • .xls → xlrd
-    • .xlsx → openpyxl (xlrd ≥2.0 dropped xlsx support)
-
-    `fp` is the stream returned by WaterButler/MFR.  It may already have been
-    read, so we always rewind and copy to an in‑memory buffer that openpyxl (and
-    ZipFile) can seek inside safely.
+    .xls → xlrd; supports truncation and optional meta collection.
     """
     sheets = OrderedDict()
     wb = xlrd.open_workbook(file_contents=to_bytes(fp))
     return parse_xls(wb, sheets)
 
 def xlsx(fp):
+    """
+    .xlsx → openpyxl; supports truncation and optional meta collection.
+    """
     sheets = OrderedDict()
     try:
         wb = load_workbook(BytesIO(to_bytes(fp)), data_only=True, read_only=True)

mfr/extensions/tabular/render.py

@@ -43,6 +43,7 @@ def render(self):
                 height=settings.TABLE_HEIGHT,
                 sheets=json.dumps(sheets),
                 options=json.dumps(size),
+                max_size=settings.MAX_SIZE,
             )
 
         assert nbr_rows and nbr_cols

mfr/extensions/tabular/templates/viewer.mako

@@ -4,10 +4,11 @@
 <link rel="stylesheet" href="${base}/css/bootstrap.min.css">
 
 <div id="mfrViewer" style="min-height: ${height}px;">
+    <div id="mfr-tabular-notice" class="alert alert-warning" style="display:none; font-size: 1.05em;"></div>
     <div class="scroller scroller-left"><i class="glyphicon glyphicon-chevron-left"></i></div>
     <div class="scroller scroller-right"><i class="glyphicon glyphicon-chevron-right"></i></div>
     <nav class="wrapper">
-        <ul id="tabular-tabs" class="nav nav-tabs list" style="height: 45px; overflow: auto; white-space: nowrap;"> 
+        <ul id="tabular-tabs" class="nav nav-tabs list" style="height: 45px; overflow: auto; white-space: nowrap;">
         </ul>
     </nav>
     <div id="inlineFilterPanel" style="background:#dddddd;padding:3px;color:black;">
@@ -30,6 +31,19 @@
         var grid;
         var data;
         var searchString = "";
+        var MAX_ROWS_LIMIT = ${max_size};
+
+        function refreshNotice() {
+            var $n = $("#mfr-tabular-notice");
+            if (!grid) { $n.hide().text(""); return; }
+            var rendered = grid.getDataLength ? grid.getDataLength() : (data ? data.length : 0);
+            if (rendered >= MAX_ROWS_LIMIT) {
+                $n.text("Table exceeds the max size limit — rendered first " + rendered + " rows.");
+                $n.show();
+            } else {
+                $n.hide().text("");
+            }
+        }
 
         for (var sheetName in sheets){
             var sheet = sheets[sheetName];
@@ -48,11 +62,13 @@
                 $("#txtSearch").value = "";
                 data = grid.getData();
                 grid.onSort.subscribe(sortData);
+                refreshNotice();
             });
         }
 
         $("#tabular-tabs").tab();
         $("#tabular-tabs a:first").click();
+        setTimeout(refreshNotice, 0);
 
         $("#txtSearch").keyup(function (e) {
             // clear on Esc
@@ -133,7 +149,7 @@
             grid.invalidate();
             grid.render();
         }
-        
+
         function reAdjust(){
             liFirstPosLeft = $('.list li:first').position().left;
             liLastPosRight = $('.list li:last').position().left + $('.list li:last').width();

mfr/extensions/tabular/utilities.py

@@ -8,13 +8,12 @@
 from tempfile import NamedTemporaryFile
 
 from mfr.extensions.tabular import compat
-from mfr.core.exceptions import SubprocessError, TooBigToRenderError, CorruptedError
+from mfr.core.exceptions import SubprocessError, CorruptedError
 from mfr.extensions.tabular.settings import (PSPP_CONVERT_BIN,
-                                             PSPP_CONVERT_TIMEOUT)
+                                             PSPP_CONVERT_TIMEOUT,
+                                             MAX_SIZE)
 
 
-MAX_SIZE = 10_000
-
 def header_population(headers):
     """make column headers from a list
     :param headers: list of column headers
@@ -128,42 +127,52 @@ def to_bytes(fp):
     return data
 
 
+def _extract_rows(fields, raw_rows):
+    rows = []
+    for row in raw_rows:
+        if len(rows) >= MAX_SIZE:
+            break
+        rows.append(dict(zip(fields, row)))
+    return rows
+
+
 def parse_xls(wb, sheets):
     for sheet in wb.sheets():
-        verify_size(sheet.nrows, sheet.ncols, '.xls')
-        fields = fix_headers(sheet.row_values(0))
-        rows = [
-            dict(zip(fields, row_vals(sheet.row(r), wb.datemode)))
+        if getattr(sheet, 'nrows', None) is None or getattr(sheet, 'ncols', None) is None:
+            raise CorruptedError
+
+        ncols = sheet.ncols
+        max_cols = min(ncols, MAX_SIZE)
+        fields = fix_headers(sheet.row_values(0)[:max_cols])
+        raw_rows = (
+            row_vals(sheet.row(r)[:max_cols], wb.datemode)
             for r in range(1, sheet.nrows)
-        ]
+        )
+        rows = _extract_rows(fields, raw_rows)
         sheets[sheet.name] = (header_population(fields), rows)
     return sheets
 
 
 def parse_xlsx(wb, sheets):
     for name in wb.sheetnames:
         ws = wb[name]
+
+        if getattr(ws, 'max_row', None) is None or getattr(ws, 'max_column', None) is None:
+            raise CorruptedError
+
+        ncols = getattr(ws, "max_column", 0)
+        max_cols = min(ncols, MAX_SIZE)
         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_SIZE,
-                                    max_col=MAX_SIZE,
-                                    values_only=True)
-        ]
+        fields = fix_headers(list(header_row)[:max_cols])
+        raw_rows = (
+            row[:max_cols] if row else []
+            for row in ws.iter_rows(min_row=2, values_only=True)
+        )
+        rows = _extract_rows(fields, raw_rows)
         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)
-
-
 def fix_headers(raw):
     return [str(v) if v not in (None, '') else f'Unnamed: {i + 1}' for i, v in enumerate(raw)]