Add H5Tdecode2, rename and deprecate H5Tdecode (#5213)

c++/src/H5DataType.cpp

@@ -250,7 +250,7 @@ DataType::p_decode() const
     }
 
     // Call C function to decode the binary object description
-    hid_t encoded_dtype_id = H5Tdecode(encoded_buf);
+    hid_t encoded_dtype_id = H5Tdecode2(encoded_buf, buf_size);
 
     // If H5Tdecode fails, raise exception
     if (encoded_dtype_id < 0) {
@@ -924,7 +924,8 @@ DataType::close()
         // Free and reset buffer of encoded object description if it's been used
         if (encoded_buf != NULL) {
             delete[] encoded_buf;
-            buf_size = 0;
+            encoded_buf = NULL;
+            buf_size    = 0;
         }
     }
 }

fortran/src/H5Tf.c

@@ -1890,47 +1890,6 @@ h5tcommitted_c(hid_t_f *dtype_id)
     return ret_value;
 }
 
-/****if* H5Tf/h5tdecode_c
- * NAME
- *  h5tdecode_c
- * PURPOSE
- *  Call H5Tdecode
- * INPUTS
- *
- *		buf     - Buffer for the data space object to be decoded.
- * OUTPUTS
- *
- *  obj_id  - Object_id (non-negative)
- *
- * RETURNS
- *  0 on success, -1 on failure
- * SOURCE
- */
-
-int_f
-h5tdecode_c(_fcd buf, hid_t_f *obj_id)
-/******/
-{
-    int            ret_value = -1;
-    unsigned char *c_buf     = NULL; /* Buffer to hold C string */
-    hid_t          c_obj_id;
-
-    /*
-     * Call H5Tdecode function.
-     */
-
-    c_buf = (unsigned char *)buf;
-
-    c_obj_id = H5Tdecode(c_buf);
-    if (c_obj_id < 0)
-        return ret_value;
-
-    *obj_id   = (hid_t_f)c_obj_id;
-    ret_value = 0;
-
-    return ret_value;
-}
-
 /****if* H5Tf/h5tencode_c
  * NAME
  *  h5tencode_c

fortran/src/H5Tff.F90

@@ -1855,32 +1855,47 @@ END SUBROUTINE h5tcommitted_f
 !>
 !! \ingroup FH5T
 !!
-!! \brief Decode A binary object description of data type and return a new object handle.
+!! \brief Decode a binary object description of data type and return a new object handle.
 !!
-!! \param buf    Buffer for the data space object to be decoded.
-!! \param obj_id Object ID.
-!! \param hdferr \fortran_error
+!! \param buf      Buffer for the data space object to be decoded.
+!! \param obj_id   Object ID.
+!! \param hdferr   \fortran_error
+!! \param buf_size Size of the buffer.
 !!
-!! See C API: @ref H5Tdecode()
+!! See C API: @ref H5Tdecode2()
 !!
-  SUBROUTINE h5tdecode_f(buf, obj_id, hdferr)
+SUBROUTINE h5tdecode_f(buf, obj_id, hdferr, buf_size)
     IMPLICIT NONE
     CHARACTER(LEN=*), INTENT(IN) :: buf
     INTEGER(HID_T), INTENT(OUT) :: obj_id
     INTEGER, INTENT(OUT) :: hdferr
+    INTEGER(SIZE_T), OPTIONAL, INTENT(IN) :: buf_size
+
+    INTEGER(SIZE_T) :: buf_size_default
+
     INTERFACE
-       INTEGER FUNCTION h5tdecode_c(buf, obj_id) BIND(C,NAME='h5tdecode_c')
+       INTEGER(HID_T) FUNCTION H5Tdecode2(buf, buf_size) BIND(C,NAME='H5Tdecode2')
          IMPORT :: C_CHAR
-         IMPORT :: HID_T
+         IMPORT :: HID_T, SIZE_T
          IMPLICIT NONE
-         CHARACTER(KIND=C_CHAR), DIMENSION(*), INTENT(IN) :: buf
-         INTEGER(HID_T), INTENT(OUT) :: obj_id
-       END FUNCTION h5tdecode_c
+         CHARACTER(KIND=C_CHAR), DIMENSION(*) :: buf
+         INTEGER(SIZE_T), VALUE :: buf_size
+       END FUNCTION H5Tdecode2
     END INTERFACE
 
-    hdferr = h5tdecode_c(buf, obj_id)
+    IF(PRESENT(buf_size))THEN
+        buf_size_default = buf_size
+    ELSE
+        buf_size_default = LEN(buf)
+    ENDIF
+
+    obj_id = H5Tdecode2(buf, buf_size_default)
+
+    IF(obj_id.LT.0)THEN
+      hdferr = -1
+    ENDIF
 
-  END SUBROUTINE h5tdecode_f
+END SUBROUTINE h5tdecode_f
 
 !>
 !! \ingroup FH5T

fortran/src/H5f90proto.h

@@ -327,7 +327,6 @@ H5_FCDLL int_f h5tvlen_create_c(hid_t_f *type_id, hid_t_f *vltype_id);
 H5_FCDLL int_f h5tis_variable_str_c(hid_t_f *type_id, int_f *flag);
 H5_FCDLL int_f h5tget_member_class_c(hid_t_f *type_id, int_f *member_no, int_f *cls);
 H5_FCDLL int_f h5tcommit_anon_c(hid_t_f *loc_id, hid_t_f *dtype_id, hid_t_f *tcpl_id, hid_t_f *tapl_id);
-H5_FCDLL int_f h5tdecode_c(_fcd buf, hid_t_f *obj_id);
 H5_FCDLL int_f h5tencode_c(_fcd buf, hid_t_f *obj_id, size_t_f *nalloc);
 H5_FCDLL int_f h5tget_create_plist_c(hid_t_f *dtype_id, hid_t_f *dtpl_id);
 H5_FCDLL int_f h5tcompiler_conv_c(hid_t_f *src_id, hid_t_f *dst_id, int_f *c_flag);

fortran/test/tH5T.F90

@@ -109,6 +109,7 @@ SUBROUTINE compoundtest(cleanup, total_error)
      CHARACTER(LEN=1024) :: cmpd_buf
      INTEGER(SIZE_T) :: cmpd_buf_size=0
      INTEGER(HID_T) :: decoded_tid1
+     INTEGER(HID_T) :: decoded_tid2
 
      INTEGER(HID_T) :: fixed_str1, fixed_str2
      LOGICAL :: are_equal
@@ -553,7 +554,10 @@ SUBROUTINE compoundtest(cleanup, total_error)
      CALL H5Tencode_f(dtype_id, cmpd_buf, cmpd_buf_size, error)
      CALL check("H5Tencode_f", error, total_error)
 
-     !  Try decoding bogus buffer
+     !  Try decoding bogus buffer with and without optional buffer size
+
+     CALL H5Tdecode_f(cmpd_buf, decoded_tid1, error, cmpd_buf_size)
+     CALL verify("H5Tdecode_f", error, -1, total_error)
 
      CALL H5Tdecode_f(cmpd_buf, decoded_tid1, error)
      CALL verify("H5Tdecode_f", error, -1, total_error)
@@ -562,14 +566,23 @@ SUBROUTINE compoundtest(cleanup, total_error)
      CALL check("H5Tencode_f", error, total_error)
 
      !  Decode from the compound buffer and return an object handle
-     CALL H5Tdecode_f(cmpd_buf, decoded_tid1, error)
+     CALL H5Tdecode_f(cmpd_buf, decoded_tid1, error, cmpd_buf_size)
      CALL check("H5Tdecode_f", error, total_error)
 
      !  Verify that the datatype was copied exactly
 
      CALL H5Tequal_f(decoded_tid1, dtype_id, flag, error)
      CALL check("H5Tequal_f", error, total_error)
      CALL verify("H5Tequal_f", flag, .TRUE., total_error)
+
+     ! Decode from the compound buffer without the optional parameter
+     CALL H5Tdecode_f(cmpd_buf, decoded_tid2, error)
+     CALL check("H5Tdecode_f", error, total_error)
+
+     !  Verify that the datatype was copied exactly
+     CALL H5Tequal_f(decoded_tid2, dtype_id, flag, error)
+     CALL check("H5Tequal_f", error, total_error)
+
      !
      ! Close all open objects.
      !

java/src/hdf/hdf5lib/H5.java

@@ -13865,16 +13865,19 @@ public static long H5Tcreate(int tclass, long size) throws HDF5LibraryException
      * @param buf
      *            IN: Buffer for the data type object to be decoded.
      *
+     * @param buf_size
+     *           IN: Size of the buffer.
+     *
      * @return a new object handle
      *
      * @exception HDF5LibraryException
      *            Error from the HDF5 Library.
      * @exception NullPointerException
      *            buf is null.
      **/
-    public static long H5Tdecode(byte[] buf) throws HDF5LibraryException, NullPointerException
+    public static long H5Tdecode(byte[] buf, long buf_size) throws HDF5LibraryException, NullPointerException
     {
-        long id = _H5Tdecode(buf);
+        long id = _H5Tdecode(buf, buf_size);
         if (id > 0) {
             log.trace("OPEN_IDS: H5Tdecode add {}", id);
             OPEN_IDS.add(id);
@@ -13883,7 +13886,7 @@ public static long H5Tdecode(byte[] buf) throws HDF5LibraryException, NullPointe
         return id;
     }
 
-    private synchronized static native long _H5Tdecode(byte[] buf)
+    private synchronized static native long _H5Tdecode(byte[] buf, long buf_size)
         throws HDF5LibraryException, NullPointerException;
 
     /**

release_docs/RELEASE.txt

@@ -180,6 +180,23 @@ New Features
       H5Iregister_type() will map to the new signature unless the library is
       explicitly configured to use an older version of the API.
 
+    - The H5Tdecode() signature has changed
+
+      When provided malformed or too-small buffers, H5Tdecode() would crash.
+      The new buffer size parameter allows this to be reliably avoided.
+
+      The old signature has been renamed to H5Tdecode1() and is considered
+      deprecated:
+
+        hid_t H5Tdecode1(const void *buf);
+
+      The new signature is H5Tdecode2(). New code should use this version:
+
+        hid_t H5Tdecode2(const void *buf, size_t buf_size);
+
+      H5Tdecode() will map to the new signature unless the library is
+      explicitly configured to use an older version of the API.
+
     - H5F_LIBVER_LATEST is now an enum value
 
       This was previously #defined to the latest H5F_libver_t API version, but

src/H5T.c

@@ -3691,7 +3691,7 @@ H5Tencode(hid_t obj_id, void *buf, size_t *nalloc)
 } /* end H5Tencode() */
 
 /*-------------------------------------------------------------------------
- * Function:  H5Tdecode
+ * Function:  H5Tdecode2
  *
  * Purpose:   Decode a binary object description and return a new object
  *            handle.
@@ -3703,7 +3703,7 @@ H5Tencode(hid_t obj_id, void *buf, size_t *nalloc)
  *-------------------------------------------------------------------------
  */
 hid_t
-H5Tdecode(const void *buf)
+H5Tdecode2(const void *buf, size_t buf_size)
 {
     H5T_t *dt;
     hid_t  ret_value; /* Return value */
@@ -3714,13 +3714,8 @@ H5Tdecode(const void *buf)
     if (buf == NULL)
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "empty buffer");
 
-    /* Create datatype by decoding buffer
-     * There is no way to get the size of the buffer, so we pass in
-     * SIZE_MAX and assume the caller knows what they are doing.
-     * Really fixing this will require an H5Tdecode2() call that
-     * takes a size parameter.
-     */
-    if (NULL == (dt = H5T_decode(SIZE_MAX, (const unsigned char *)buf)))
+    /* Create datatype by decoding buffer */
+    if (NULL == (dt = H5T_decode(buf_size, buf)))
         HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, H5I_INVALID_HID, "can't decode object");
 
     /* Register the type and return the ID */
@@ -3729,7 +3724,7 @@ H5Tdecode(const void *buf)
 
 done:
     FUNC_LEAVE_API(ret_value)
-} /* end H5Tdecode() */
+} /* end H5Tdecode2() */
 
 /*-------------------------------------------------------------------------
  * API functions are above; library-private functions are below...
@@ -3812,10 +3807,16 @@ H5T_decode(size_t buf_size, const unsigned char *buf)
     if (NULL == (f = H5F_fake_alloc((uint8_t)0)))
         HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, NULL, "can't allocate fake file struct");
 
+    if (buf_size != SIZE_MAX && H5_IS_BUFFER_OVERFLOW(buf, 1, buf + buf_size - 1))
+        HGOTO_ERROR(H5E_DATATYPE, H5E_BADMESG, NULL, "buffer too small to be datatype message");
+
     /* Decode the type of the information */
     if (*buf++ != H5O_DTYPE_ID)
         HGOTO_ERROR(H5E_DATATYPE, H5E_BADMESG, NULL, "not an encoded datatype");
 
+    if (buf_size != SIZE_MAX && H5_IS_BUFFER_OVERFLOW(buf, 1, buf + buf_size - 1))
+        HGOTO_ERROR(H5E_DATATYPE, H5E_BADMESG, NULL, "buffer too small to be datatype message");
+
     /* Decode the version of the datatype information */
     if (*buf++ != H5T_ENCODE_VERSION)
         HGOTO_ERROR(H5E_DATATYPE, H5E_VERSION, NULL, "unknown version of encoded datatype");

src/H5Tdeprec.c

@@ -187,4 +187,48 @@ H5Topen1(hid_t loc_id, const char *name)
 
     FUNC_LEAVE_API(ret_value)
 } /* end H5Topen1() */
+
+/*-------------------------------------------------------------------------
+ * Function:  H5Tdecode1
+ *
+ * Purpose:   Decode a binary object description and return a new object
+ *            handle.
+ *
+ * Note:      Deprecated in favor of H5Tdecode2
+ *
+ * Return:    Success:    datatype ID(non-negative)
+ *
+ *            Failure:    negative
+ *
+ *-------------------------------------------------------------------------
+ */
+hid_t
+H5Tdecode1(const void *buf)
+{
+    H5T_t *dt;
+    hid_t  ret_value; /* Return value */
+
+    FUNC_ENTER_API(H5I_INVALID_HID)
+
+    /* Check args */
+    if (buf == NULL)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "empty buffer");
+
+    /* Create datatype by decoding buffer
+     * There is no way to get the size of the buffer, so we pass in
+     * SIZE_MAX and assume the caller knows what they are doing.
+     * Really fixing this will require an H5Tdecode2() call that
+     * takes a size parameter.
+     */
+    if (NULL == (dt = H5T_decode(SIZE_MAX, (const unsigned char *)buf)))
+        HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, H5I_INVALID_HID, "can't decode object");
+
+    /* Register the type and return the ID */
+    if ((ret_value = H5I_register(H5I_DATATYPE, dt, true)) < 0)
+        HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register data type");
+
+done:
+    FUNC_LEAVE_API(ret_value)
+} /* end H5Tdecode1() */
+
 #endif /* H5_NO_DEPRECATED_SYMBOLS */