Merge pull request #11 from us-irs/refactor-and-fix-crc-handling

Refactor and fix crc handling

CHANGELOG.md

@@ -13,11 +13,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## Fixed
 
 - The large file flag was not set properly in the source handler for large file transfers.
+- The CRC algorithms will now be used for empty files as well instead of hardcoding the
+  checksum type to the NULL checksum. This was a bug which did not show directly for
+  checksums like CRC32 because those have an initial value of 0x0
 
 ## Changed
 
 - Added `file_size` abstract method to `VirtualFilestore`
 - Renamed `HostFilestore` to `NativeFilestore`, but keep old name alias for backwards compatibility.
+- Added `calculate_checksum` and `verify_checksum` to `VirtualFilestore` interface.
 
 # [v0.1.2] 2024-06-04
 

docs/api/cfdp.rst

@@ -50,6 +50,14 @@ Exceptions Module
    :undoc-members:
    :show-inheritance:
 
+CRC Module
+-------------------------------------------
+
+.. automodule:: cfdppy.crc
+   :members:
+   :undoc-members:
+   :show-inheritance:
+
 Definitions Module
 --------------------------
 

docs/conf.py

@@ -32,7 +32,9 @@
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
 extensions = [
+    "sphinx.ext.autosectionlabel",
     "sphinx.ext.autodoc",
+    "sphinx.ext.napoleon",
     "sphinx.ext.intersphinx",
     "sphinx.ext.doctest",
     "sphinx_rtd_theme",

src/cfdppy/crc.py

@@ -0,0 +1,19 @@
+import struct
+from pathlib import Path
+
+
+def calc_modular_checksum(file_path: Path) -> bytes:
+    """Calculates the modular checksum for a file in one go."""
+    checksum = 0
+
+    with open(file_path, "rb") as file:
+        while True:
+            data = file.read(4)
+            if not data:
+                break
+            checksum += int.from_bytes(
+                data.ljust(4, b"\0"), byteorder="big", signed=False
+            )
+
+    checksum %= 2**32
+    return struct.pack("!I", checksum)

src/cfdppy/filestore.py

@@ -6,7 +6,12 @@
 from pathlib import Path
 from typing import Optional, BinaryIO
 
+from cfdppy.crc import calc_modular_checksum
+from crcmod.predefined import PredefinedCrc
+from spacepackets.cfdp.defs import NULL_CHECKSUM_U32, ChecksumType
 from spacepackets.cfdp.tlv import FilestoreResponseStatusCode
+from cfdppy.exceptions import ChecksumNotImplemented
+
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -101,6 +106,41 @@ def list_directory(
         _LOGGER.warning("Listing directory not implemented in virtual filestore")
         return FilestoreResponseStatusCode.NOT_PERFORMED
 
+    @abc.abstractmethod
+    def calculate_checksum(
+        self,
+        checksum_type: ChecksumType,
+        file_path: Path,
+        size_to_verify: int,
+        segment_len: int = 4096,
+    ) -> bytes:
+        """Calculate the checksum for a given file.
+
+        Raises
+        -------
+
+        ValueError
+            Invalid input parameters
+        FileNotFoundError
+            File for checksum calculation does not exist
+        """
+        pass
+
+    def verify_checksum(
+        self,
+        checksum: bytes,
+        checksum_type: ChecksumType,
+        file_path: Path,
+        size_to_verify: int,
+        segment_len: int = 4096,
+    ) -> bool:
+        return (
+            self.calculate_checksum(
+                checksum_type, file_path, size_to_verify, segment_len
+            )
+            == checksum
+        )
+
 
 class NativeFilestore(VirtualFilestore):
     def __init__(self):
@@ -265,5 +305,51 @@ def list_directory(
             os.chdir(curr_path)
         return FilestoreResponseStatusCode.SUCCESS
 
+    def _verify_checksum(self, checksum_type: ChecksumType):
+        if checksum_type not in [
+            ChecksumType.CRC_32,
+            ChecksumType.CRC_32C,
+        ]:
+            raise ChecksumNotImplemented(checksum_type)
+
+    def checksum_type_to_crcmod_str(self, checksum_type: ChecksumType) -> Optional[str]:
+        if checksum_type == ChecksumType.CRC_32:
+            return "crc32"
+        elif checksum_type == ChecksumType.CRC_32C:
+            return "crc32c"
+        raise ChecksumNotImplemented(checksum_type)
+
+    def _generate_crc_calculator(self, checksum_type: ChecksumType) -> PredefinedCrc:
+        self._verify_checksum(checksum_type)
+        return PredefinedCrc(self.checksum_type_to_crcmod_str(checksum_type))
+
+    def calculate_checksum(
+        self,
+        checksum_type: ChecksumType,
+        file_path: Path,
+        file_sz: int,
+        segment_len: int = 4096,
+    ) -> bytes:
+        if checksum_type == ChecksumType.NULL_CHECKSUM:
+            return NULL_CHECKSUM_U32
+        if not file_path.exists():
+            raise FileNotFoundError(file_path)
+        if checksum_type == ChecksumType.MODULAR:
+            return calc_modular_checksum(file_path)
+        if segment_len == 0:
+            raise ValueError("segment length can not be 0")
+        crc_obj = self._generate_crc_calculator(checksum_type)
+        current_offset = 0
+        # Calculate the file CRC
+        with open(file_path, "rb") as file:
+            while current_offset < file_sz:
+                read_len = min(segment_len, file_sz - current_offset)
+                if read_len > 0:
+                    crc_obj.update(
+                        self.read_from_opened_file(file, current_offset, read_len)
+                    )
+                current_offset += read_len
+            return crc_obj.digest()
+
 
 HostFilestore = NativeFilestore

src/cfdppy/handler/dest.py

@@ -23,7 +23,6 @@
     _PositiveAckProcedureParams,
     get_packet_destination,
 )
-from cfdppy.handler.crc import CrcHelper
 from cfdppy.handler.defs import (
     _FileParamsBase,
 )
@@ -240,6 +239,7 @@ def __init__(self):
         self.check_timer: Optional[Countdown] = None
         self.current_check_count: int = 0
         self.closure_requested: bool = False
+        self.checksum_type: ChecksumType = ChecksumType.NULL_CHECKSUM
         self.finished_params: FinishedParams = FinishedParams(
             delivery_code=DeliveryCode.DATA_INCOMPLETE,
             file_status=FileStatus.FILE_STATUS_UNREPORTED,
@@ -255,23 +255,6 @@ def __init__(self):
         self.positive_ack_params = _PositiveAckProcedureParams()
         self.last_inserted_packet = PduHolder(None)
 
-    def reset(self):
-        self.transaction_id = None
-        self.closure_requested = False
-        self.pdu_conf = PduConfig.empty()
-        self.finished_params = FinishedParams(
-            condition_code=ConditionCode.NO_ERROR,
-            delivery_code=DeliveryCode.DATA_INCOMPLETE,
-            file_status=FileStatus.FILE_STATUS_UNREPORTED,
-        )
-        self.finished_params.file_status = FileStatus.FILE_STATUS_UNREPORTED
-        self.completion_disposition = CompletionDisposition.COMPLETED
-        self.fp.reset()
-        self.acked_params = _AckedModeParams()
-        self.remote_cfg = None
-        self.last_inserted_packet.pdu = None
-        self.current_check_count = 0
-
 
 class FsmResult:
     def __init__(self, states: DestStateWrapper):
@@ -339,9 +322,6 @@ def __init__(
         self.user = user
         self.check_timer_provider = check_timer_provider
         self._params = _DestFieldWrapper()
-        self._cksum_verif_helper: CrcHelper = CrcHelper(
-            ChecksumType.NULL_CHECKSUM, user.vfs
-        )
         self._pdus_to_be_sent: Deque[PduHolder] = deque()
 
     @property
@@ -499,7 +479,7 @@ def reset(self):
         """This function is public to allow completely resetting the handler, but it is explicitely
         discouraged to do this. CFDP generally has mechanism to detect issues and errors on itself.
         """
-        self._params.reset()
+        self._params = _DestFieldWrapper()
         self._pdus_to_be_sent.clear()
         self.states.state = CfdpState.IDLE
         self.states.step = TransactionStep.IDLE
@@ -596,7 +576,7 @@ def _fsm_advancement_after_packets_were_sent(self):
     def _start_transaction(self, metadata_pdu: MetadataPdu) -> bool:
         if self.states.state != CfdpState.IDLE:
             return False
-        self._params.reset()
+        self._params = _DestFieldWrapper()
         self._common_first_packet_handler(metadata_pdu)
         self._handle_metadata_packet(metadata_pdu)
         return True
@@ -686,7 +666,7 @@ def _handle_fd_without_previous_metadata(
                 )
 
     def _common_first_packet_not_metadata_pdu_handler(self, pdu: GenericPduPacket):
-        self._params.reset()
+        self._params = _DestFieldWrapper()
         self._common_first_packet_handler(pdu)
         self.states.step = TransactionStep.WAITING_FOR_METADATA
         self._params.acked_params.metadata_missing = True
@@ -705,7 +685,7 @@ def _common_first_packet_handler(self, pdu: GenericPduPacket):
         self._params.remote_cfg = self.remote_cfg_table.get_cfg(pdu.source_entity_id)
 
     def _handle_metadata_packet(self, metadata_pdu: MetadataPdu):
-        self._cksum_verif_helper.checksum_type = metadata_pdu.checksum_type
+        self._params.checksum_type = metadata_pdu.checksum_type
         self._params.closure_requested = metadata_pdu.closure_requested
         self._params.acked_params.metadata_missing = False
         if metadata_pdu.dest_file_name is None or metadata_pdu.source_file_name is None:
@@ -1090,13 +1070,15 @@ def _prepare_eof_ack_packet(self):
     def _checksum_verify(self) -> bool:
         file_delivery_complete = False
         if (
-            self._cksum_verif_helper.checksum_type == ChecksumType.NULL_CHECKSUM
+            self._params.checksum_type == ChecksumType.NULL_CHECKSUM
             or self._params.fp.metadata_only
         ):
             file_delivery_complete = True
         else:
-            crc32 = self._cksum_verif_helper.calc_for_file(
-                self._params.fp.file_name, self._params.fp.progress
+            crc32 = self.user.vfs.calculate_checksum(
+                self._params.checksum_type,
+                self._params.fp.file_name,
+                self._params.fp.progress,
             )
             if crc32 == self._params.fp.crc32:
                 file_delivery_complete = True