Repository: pydap/pydap (python)

Issue:
add CRC32_checksum
Hyrax and Thredds include a flag in their dap responses (dap4) to compute checksums. These are added to the dap response as an element of the dmr portion, and the last 4 digits. pydap already checks for this element and parses it:

https://github.com/pydap/pydap/blob/98cdac8432358f4beda9e1daccfc8a08ae4e48d0/src/pydap/handlers/dap.py#L996-L1000


Both of the OPeNDAP servers implement CRC32_checksum. However, pydap does not compute and recheck that the value of the checksum matches that included in the dap response. 

Pydap should conpute this checksum and the user should be able to set this option (i.e. compute/evaluate checksums) an optional argument when using pydap (`False `by default)!



Pull request:
Enable option to requests/skip checksums
<!-- Thank you very much for your hard work and contribution! -->

The following Pull Request:

- [x] Closes #516 
- [x] Tests are added to demonstrate the new or fixed behavior, and all tests pass
on a local environment.


Patch:
diff --git a/src/pydap/client.py b/src/pydap/client.py
index a902a59..c02868c 100644
--- a/src/pydap/client.py
+++ b/src/pydap/client.py
@@ -78,6 +78,7 @@ def open_url(
     output_grid=False,
     timeout=DEFAULT_TIMEOUT,
     verify=True,
+    checksum=False,
     user_charset="ascii",
     protocol=None,
     use_cache=False,
@@ -143,6 +144,7 @@ def open_url(
         output_grid,
         timeout=timeout,
         verify=verify,
+        checksum=checksum,
         user_charset=user_charset,
         protocol=protocol,
         get_kwargs=get_kwargs,
diff --git a/src/pydap/handlers/dap.py b/src/pydap/handlers/dap.py
index 87f698a..a391e92 100644
--- a/src/pydap/handlers/dap.py
+++ b/src/pydap/handlers/dap.py
@@ -66,6 +66,7 @@ class DAPHandler(BaseHandler):
         output_grid=True,
         timeout=DEFAULT_TIMEOUT,
         verify=True,
+        checksum=False,
         user_charset="ascii",
         protocol=None,
         get_kwargs=None,
@@ -76,6 +77,7 @@ class DAPHandler(BaseHandler):
         self.output_grid = output_grid
         self.timeout = timeout
         self.verify = verify
+        self.checksum = checksum
         self.user_charset = user_charset
         self.url = url
 
@@ -246,6 +248,7 @@ class DAPHandler(BaseHandler):
                 session=self.session,
                 timeout=self.timeout,
                 verify=self.verify,
+                checksum=self.checksum,
                 get_kwargs={**self.get_kwargs, "stream": True},
             )
 
@@ -459,6 +462,7 @@ class BaseProxyDap4(BaseProxyDap2):
         session=None,
         timeout=DEFAULT_TIMEOUT,
         verify=True,
+        checksum=False,
         user_charset="ascii",
         get_kwargs=None,
     ):
@@ -471,6 +475,7 @@ class BaseProxyDap4(BaseProxyDap2):
         self.session = session
         self.timeout = timeout
         self.verify = verify
+        self.checksum = checksum
         self.user_charset = user_charset
         self.get_kwargs = get_kwargs or {}
 
@@ -485,7 +490,10 @@ class BaseProxyDap4(BaseProxyDap2):
         scheme, netloc, path, _, query, fragment = urlparse(self.baseurl)
         ce = "dap4.ce=" + self.id + hyperslab(index)
         url = urlunparse((scheme, netloc, path + ".dap", "", ce, fragment)).rstrip("&")
-
+        if self.checksum:
+            url += "&dap4.checksum=true"
+        else:
+            url += "&dap4.checksum=false"
         # download and unpack data
         logger.info("Fetching URL: %s" % url)
 
@@ -498,9 +506,11 @@ class BaseProxyDap4(BaseProxyDap2):
             get_kwargs=self.get_kwargs,
         )
 
-        dataset = UNPACKDAP4DATA(r, self.user_charset).dataset
-        # self.checksum = dataset[self.id].attributes["checksum"]
+        dataset = UNPACKDAP4DATA(r, self.checksum, self.user_charset).dataset
         self.data = dataset[self.id].data
+        if self.checksum:
+            self.checksum = dataset[self.id].attributes["_DAP4_Checksum_CRC32"]
+
         return self.data
 
 
@@ -933,8 +943,9 @@ class UNPACKDAP4DATA(object):
             See `pydap.net.get.open_dap_file`
     """
 
-    def __init__(self, r, user_charset="ascii"):
+    def __init__(self, r, checksum=False, user_charset="ascii"):
         self.user_charset = user_charset
+        self.checksum = checksum
         if isinstance(r, requests.Response):
             # remote dataset
             self.r = r
@@ -1017,11 +1028,14 @@ class UNPACKDAP4DATA(object):
                 variable=variable,
                 endian=self.endianness,
             )
-            checksum = numpy.frombuffer(
-                buffer[stop : stop + 4], dtype=checksum_dtype
-            ).byteswap("=")
             variable._set_data(data)
-            variable.attributes["checksum"] = checksum
+
+            if self.checksum:
+                checksum = numpy.frombuffer(
+                    buffer[stop : stop + 4], dtype=checksum_dtype
+                )
+                variable.attributes["_DAP4_Checksum_CRC32"] = checksum[0]
+
             # Jump over the 4 byte chunk_header
             start = stop + 4
         return dataset
diff --git a/src/pydap/model.py b/src/pydap/model.py
index a8d003e..879d083 100644
--- a/src/pydap/model.py
+++ b/src/pydap/model.py
@@ -426,9 +426,10 @@ class BaseType(DapType):
                 pass  # Leave as-is for types that don't support __array__
         out.data = data
         if type(self.data).__name__ == "BaseProxyDap4":
-            # out.attributes["checksum"] = self.data.checksum
-            out.attributes["Maps"] = self.Maps
-
+            if self.data.checksum:
+                # updates it if defined
+                out.attributes["_DAP4_Checksum_CRC32"] = self.data.checksum
+            out.attributes.update({"Maps": self.Maps})
         return out
 
     def __len__(self):


Test patch:
diff --git a/src/pydap/tests/test_handlers_dap.py b/src/pydap/tests/test_handlers_dap.py
index 9d3b669..c9efe71 100644
--- a/src/pydap/tests/test_handlers_dap.py
+++ b/src/pydap/tests/test_handlers_dap.py
@@ -29,6 +29,8 @@ from pydap.tests.datasets import (
     VerySimpleSequence,
 )
 
+from ..net import create_session
+
 try:
     from unittest.mock import patch
 except ImportError:
@@ -714,3 +716,22 @@ def test_dap_handler_string_array():
         repr(pyds["bears"][:]) == "<BaseType with data array(shape=(2, 3), dtype=|S3)>"
     )
     np.testing.assert_equal(pyds["bears"][:].data, actual_data)
+
+
+@pytest.mark.parametrize("checksum", [True, False])
+def test_checksum(checksum):
+    """Test that the checksum if applied correctly, when requests"""
+    url = "dap4://test.opendap.org/opendap/dap4/SimpleGroup.nc4.h5"
+    session = create_session()
+    pyds = DAPHandler(url, session=session, checksum=checksum).dataset
+    Y = pyds["SimpleGroup/Y"][:]
+    if not checksum:
+        assert not hasattr(Y, "_DAP4_Checksum_CRC32")
+    else:
+        import zlib
+
+        for var in walk(pyds, BaseType):
+            assert hasattr(var[:], "_DAP4_Checksum_CRC32")
+            buffer = bytearray(var[:].data)
+            checksum = zlib.crc32(buffer)
+            assert var[:].attributes["_DAP4_Checksum_CRC32"] == checksum