Removed support for .pkl files passed to add_font() + pickle module i…

…s no more used - close #345 (#402)

* Removed support for .pkl files passed to add_font() + pickle module is no more used - close #345

* Fixing test_add_font_non_existing_file_pkl

* Updating documentation

.banditrc.yml

@@ -2,11 +2,3 @@ skips:
   # Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   # => OK, we don't care though
   - B101
-
-  # Pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue.
-  # cf. https://github.com/PyFPDF/fpdf2/issues/345
-  - B301
-
-  # Consider possible security implications associated with pickle module.
-  # cf. https://github.com/PyFPDF/fpdf2/issues/345
-  - B403

.gitignore

@@ -15,7 +15,6 @@ verapdf-aggregated.json
 contributors/contributors.html
 
 *.py[cod]
-*.pkl
 
 # C extensions
 *.so

CHANGELOG.md

@@ -22,6 +22,8 @@ This can also be enabled programmatically with `warnings.simplefilter('default',
 - some documentation on how to embed Matplotlib charts: [Maths](https://pyfpdf.github.io/fpdf2/Maths.html)
 ### Fixed
 - infinite loop when calling `.multi_cell()` without enough horizontal space - _cf._ [#389](https://github.com/PyFPDF/fpdf2/issues/389)
+### Removed
+- support for `.pkl` passed to `add_font()`. This was deprecated since v2.5.1.
 
 ## [2.5.2] - 2022-04-13
 ### Added

README.md

@@ -74,7 +74,7 @@ Features
  * Optional basic Markdown-like styling: `**bold**, __italics__, --underlined--`
  * Unit tests with `qpdf`-based PDF diffing and a high code coverage
 
-We validate all our PDF samples using 3 different checkers:
+Our 300+ reference PDF test files, generated by `fpdf2`, are validated using 3 different checkers:
 
 [![QPDF logo](https://pyfpdf.github.io/fpdf2/qpdf-logo.svg)](https://github.com/qpdf/qpdf)
 [![PDF Checker logo](https://pyfpdf.github.io/fpdf2/pdfchecker-logo.png)](https://www.datalogics.com/products/pdf-tools/pdf-checker/)

docs/Unicode.md

@@ -120,32 +120,6 @@ pdf.output("unicode.pdf")
 View the result here: 
 [unicode.pdf](https://github.com/PyFPDF/fpdf2/raw/master/tutorial/unicode.pdf)
 
-## Metric Files ##
-
-FPDF will try to automatically generate metrics (i.e. character widths) about 
-TTF font files to speed up their processing.
-
-Such metrics are stored using the Python Pickle format (`.pkl` extension), by 
-default in the font directory (ensure read and write permission!). Additional 
-information about the caching mechanism is defined in the
-[`add_font`](fpdf/fpdf.html#fpdf.fpdf.FPDF.add_font) method documentation.
-
-TTF metric files often weigh about 650K, so keep that in mind if you use many
-TTF fonts and have disk size or memory limitations.
-
-By design, metric files are not imported as they could cause a temporary memory
-leak if not managed properly (this could be an issue in a webserver environment
-with many processes or threads, so the current implementation discards metrics when
-FPDF objects are disposed).
-
-In most circumstances, you will not notice any difference about storing metric
-files vs. generating them in each run on-the-fly (according basic tests, elapsed
-time is equivalent; YMMV).
-
-Like the original PHP implementation, this library should work even if it could
-not store the metric file, and as no source code file is generated at runtime,
-it should work in restricted environments.
-
 ## Free Font Pack and Copyright Restrictions ##
 
 For your convenience, this library collected 96 TTF files in an optional 

fpdf/fpdf.py

@@ -19,7 +19,6 @@
 import logging
 import math
 import os
-import pickle
 import re
 import sys
 import warnings
@@ -29,6 +28,7 @@
 from contextlib import contextmanager
 from datetime import datetime
 from functools import wraps
+from os.path import splitext
 from pathlib import Path
 from typing import Callable, List, NamedTuple, Optional, Union
 
@@ -1502,10 +1502,6 @@ def add_font(self, family, style="", fname=None, uni="DEPRECATED"):
         Imports a TrueType or OpenType font and makes it available
         for later calls to the `set_font()` method.
 
-        **Warning:** there is partial support for Type1 and legacy fonts in .pkl font definition files,
-        generated by the `MakeFont` utility, but this feature is getting deprecated in favour of TrueType Unicode font support
-        (whose fonts are automatically processed with the included `ttfonts.py` module).
-
         You will find more information on the "Unicode" documentation page.
 
         Args:
@@ -1515,16 +1511,21 @@ def add_font(self, family, style="", fname=None, uni="DEPRECATED"):
                 If the file is not found, it will be searched in `FPDF_FONT_DIR`.
             uni (bool): [**DEPRECATED**] unused
         """
+        if not fname:
+            raise ValueError('"fname" parameter is required')
+        ext = splitext(str(fname))[1]
+        if ext not in (".otf", ".otc", ".ttf", ".ttc"):
+            raise ValueError(
+                f"Unsupported font file extension: {ext}."
+                " add_font() used to accept .pkl file as input, but for security reasons"
+                " this feature is deprecated since v2.5.1 and has been removed in v2.5.3."
+            )
         if uni != "DEPRECATED":
             warnings.warn(
                 '"uni" parameter is deprecated, unused and will soon be removed',
                 DeprecationWarning,
                 stacklevel=2,
             )
-        else:
-            uni = str(fname).endswith(".ttf")
-        if not fname:
-            raise ValueError('"fname" parameter is required')
         style = "".join(sorted(style.upper()))
         if any(letter not in "BI" for letter in style):
             raise ValueError(
@@ -1536,101 +1537,66 @@ def add_font(self, family, style="", fname=None, uni="DEPRECATED"):
         if fontkey in self.fonts or fontkey in self.core_fonts:
             warnings.warn(f"Core font or font already added '{fontkey}': doing nothing")
             return
-        if uni:
-            for parent in (".", FPDF_FONT_DIR):
-                if not parent:
-                    continue
-                if (Path(parent) / fname).exists():
-                    ttffilename = Path(parent) / fname
-                    break
-            else:
-                raise FileNotFoundError(f"TTF Font file not found: {fname}")
-
-            # include numbers in the subset! (if alias present)
-            # ensure that alias is mapped 1-by-1 additionally (must be replaceable)
-            sbarr = "\x00 "
-            if self.str_alias_nb_pages:
-                sbarr += "0123456789"
-                sbarr += self.str_alias_nb_pages
-
-            ttf = TTFontFile()
-            ttf.getMetrics(ttffilename)
-            desc = {
-                "Ascent": round(ttf.ascent),
-                "Descent": round(ttf.descent),
-                "CapHeight": round(ttf.capHeight),
-                "Flags": ttf.flags,
-                "FontBBox": (
-                    f"[{ttf.bbox[0]:.0f} {ttf.bbox[1]:.0f}"
-                    f" {ttf.bbox[2]:.0f} {ttf.bbox[3]:.0f}]"
-                ),
-                "ItalicAngle": int(ttf.italicAngle),
-                "StemV": round(ttf.stemV),
-                "MissingWidth": round(ttf.defaultWidth),
-            }
+        for parent in (".", FPDF_FONT_DIR):
+            if not parent:
+                continue
+            if (Path(parent) / fname).exists():
+                ttffilename = Path(parent) / fname
+                break
+        else:
+            raise FileNotFoundError(f"TTF Font file not found: {fname}")
 
-            # Generate metrics .pkl file
-            font_dict = {
-                "type": "TTF",
-                "name": re.sub("[ ()]", "", ttf.fullName),
-                "desc": desc,
-                "up": round(ttf.underlinePosition),
-                "ut": round(ttf.underlineThickness),
-                "ttffile": ttffilename,
-                "fontkey": fontkey,
-                "originalsize": os.stat(ttffilename).st_size,
-                "cw": ttf.charWidths,
-            }
+        # include numbers in the subset! (if alias present)
+        # ensure that alias is mapped 1-by-1 additionally (must be replaceable)
+        sbarr = "\x00 "
+        if self.str_alias_nb_pages:
+            sbarr += "0123456789"
+            sbarr += self.str_alias_nb_pages
+
+        ttf = TTFontFile()
+        ttf.getMetrics(ttffilename)
+        desc = {
+            "Ascent": round(ttf.ascent),
+            "Descent": round(ttf.descent),
+            "CapHeight": round(ttf.capHeight),
+            "Flags": ttf.flags,
+            "FontBBox": (
+                f"[{ttf.bbox[0]:.0f} {ttf.bbox[1]:.0f}"
+                f" {ttf.bbox[2]:.0f} {ttf.bbox[3]:.0f}]"
+            ),
+            "ItalicAngle": int(ttf.italicAngle),
+            "StemV": round(ttf.stemV),
+            "MissingWidth": round(ttf.defaultWidth),
+        }
 
-            self.fonts[fontkey] = {
-                "i": len(self.fonts) + 1,
-                "type": font_dict["type"],
-                "name": font_dict["name"],
-                "desc": font_dict["desc"],
-                "up": font_dict["up"],
-                "ut": font_dict["ut"],
-                "cw": font_dict["cw"],
-                "ttffile": font_dict["ttffile"],
-                "fontkey": fontkey,
-                "subset": SubsetMap(map(ord, sbarr)),
-            }
-            self.font_files[fontkey] = {
-                "length1": font_dict["originalsize"],
-                "type": "TTF",
-                "ttffile": ttffilename,
-            }
-        else:
-            warnings.warn(
-                "Support for .pkl font files definition is deprecated, and will be removed from fpdf2 soon."
-                " If you require this feature, please report your need on fpdf2 GitHub project.",
-                DeprecationWarning,
-                stacklevel=2,
-            )
-            font_dict = pickle.loads(Path(fname).read_bytes())
-            font_dict["i"] = len(self.fonts) + 1
-            self.fonts[fontkey] = font_dict
-            diff = font_dict.get("diff")
-            if diff:
-                # Search existing encodings
-                nb = len(self.diffs)
-                for i in range(1, nb + 1):
-                    if self.diffs[i] == diff:
-                        d = i
-                        break
-                else:
-                    d = nb + 1
-                    self.diffs[d] = diff
-                self.fonts[fontkey]["diff"] = d
-            filename = font_dict.get("filename")
-            if filename:
-                if font_dict["type"] == "TrueType":
-                    originalsize = font_dict["originalsize"]
-                    self.font_files[filename] = {"length1": originalsize}
-                else:
-                    self.font_files[filename] = {
-                        "length1": font_dict["size1"],
-                        "length2": font_dict["size2"],
-                    }
+        font_dict = {
+            "type": "TTF",
+            "name": re.sub("[ ()]", "", ttf.fullName),
+            "desc": desc,
+            "up": round(ttf.underlinePosition),
+            "ut": round(ttf.underlineThickness),
+            "ttffile": ttffilename,
+            "fontkey": fontkey,
+            "originalsize": os.stat(ttffilename).st_size,
+            "cw": ttf.charWidths,
+        }
+        self.fonts[fontkey] = {
+            "i": len(self.fonts) + 1,
+            "type": font_dict["type"],
+            "name": font_dict["name"],
+            "desc": font_dict["desc"],
+            "up": font_dict["up"],
+            "ut": font_dict["ut"],
+            "cw": font_dict["cw"],
+            "ttffile": font_dict["ttffile"],
+            "fontkey": fontkey,
+            "subset": SubsetMap(map(ord, sbarr)),
+        }
+        self.font_files[fontkey] = {
+            "length1": font_dict["originalsize"],
+            "type": "TTF",
+            "ttffile": ttffilename,
+        }
 
     def set_font(self, family=None, style="", size=0):
         """

test/end_to_end_legacy/charmap/test_charmap.py

@@ -62,6 +62,3 @@ def test_first_999_chars(font_filename, tmp_path):
             break
 
     assert_pdf_equal(pdf, HERE / f"charmap_first_999_chars-{font_name}.pdf", tmp_path)
-
-    for pkl_path in HERE.glob("*.pkl"):
-        pkl_path.unlink()

test/fonts/test_add_font.py

@@ -1,5 +1,3 @@
-import os
-from contextlib import suppress
 from pathlib import Path
 
 import pytest
@@ -14,16 +12,17 @@ def test_add_font_non_existing_file():
     pdf = FPDF()
     with pytest.raises(FileNotFoundError) as error:
         pdf.add_font("MyFont", fname="non-existing-file.ttf")
-    expected_msg = "TTF Font file not found: non-existing-file.ttf"
-    assert str(error.value) == expected_msg
+    assert str(error.value) == "TTF Font file not found: non-existing-file.ttf"
 
 
-def test_add_font_non_existing_file_pkl():
+def test_add_font_pkl():
     pdf = FPDF()
-    with pytest.raises(FileNotFoundError) as error:
+    with pytest.raises(ValueError) as error:
         pdf.add_font("MyFont", fname="non-existing-file.pkl")
-    expected_msg = "[Errno 2] No such file or directory: 'non-existing-file.pkl'"
-    assert str(error.value) == expected_msg
+    assert str(error.value) == (
+        "Unsupported font file extension: .pkl. add_font() used to accept .pkl file as input, "
+        "but for security reasons this feature is deprecated since v2.5.1 and has been removed in v2.5.3."
+    )
 
 
 def test_deprecation_warning_for_FPDF_CACHE_DIR():
@@ -69,12 +68,6 @@ def test_add_font_with_str_fname_ok(tmp_path):
             assert_pdf_equal(pdf, HERE / "add_font_unicode.pdf", tmp_path)
 
 
-def teardown():
-    # Clean-up for test_add_font_from_pkl
-    with suppress(FileNotFoundError):
-        os.remove("Roboto-Regular.pkl")
-
-
 def test_add_core_fonts():
     font_file_path = HERE / "Roboto-Regular.ttf"
     pdf = FPDF()

test/text/test_multi_cell.py

@@ -114,15 +114,16 @@ def test_ln_positioning_and_page_breaking_for_multicell(tmp_path):
     )
 
 
-def test_multi_cell_ln_0(tmp_path):
+def test_multi_cell_border_thickness(tmp_path):
     doc = FPDF()
     doc.add_page()
     doc.set_font("helvetica", size=TEXT_SIZE)
+    doc.set_line_width(3)
     doc.multi_cell(w=45, h=LINE_HEIGHT, border=1, txt="Lorem")
     doc.multi_cell(w=45, h=LINE_HEIGHT, border=1, txt="ipsum")
     doc.multi_cell(w=45, h=LINE_HEIGHT, border=1, txt="Ut")
     doc.multi_cell(w=45, h=LINE_HEIGHT, border=1, txt="nostrud")
-    assert_pdf_equal(doc, HERE / "multi_cell_ln_0.pdf", tmp_path)
+    assert_pdf_equal(doc, HERE / "multi_cell_border_thickness.pdf", tmp_path)
 
 
 def test_multi_cell_ln_1(tmp_path):