diff --git a/lib/include/resdata/rd_kw.hpp b/lib/include/resdata/rd_kw.hpp index fbf486ee6..23270eec6 100644 --- a/lib/include/resdata/rd_kw.hpp +++ b/lib/include/resdata/rd_kw.hpp @@ -250,6 +250,10 @@ inline rd_kw_ptr make_rd_kw(const char *header, int size, } inline std::string rd_kw_iget_stripped_string(const rd_kw_type *kw, int index) { - return rd::strip_spaces( - static_cast(rd_kw_iget_ptr(kw, index))); + const char *raw = static_cast(rd_kw_iget_ptr(kw, index)); + const size_t width = rd_type_get_sizeof_iotype(rd_kw_get_data_type(kw)); + size_t len = 0; + while (len < width && raw[len] != '\0') + len++; + return rd::strip_spaces(std::string(raw, len)); } diff --git a/lib/resdata/rd_file_view.cpp b/lib/resdata/rd_file_view.cpp index 5ad8c6bea..acd7f20ea 100644 --- a/lib/resdata/rd_file_view.cpp +++ b/lib/resdata/rd_file_view.cpp @@ -152,6 +152,11 @@ void rd_file_view_index_fload_kw(const rd_file_view_type *rd_file_view, char *io_buffer) { auto file_kw = rd_file_view_iget_named_file_kw(rd_file_view, kw, index); + if (!file_kw) + throw std::invalid_argument(std::string("Keyword '") + kw + "' index " + + std::to_string(index) + + " not found in file view"); + if (rd_file_view->fortio->assert_stream_open()) { offset_type offset = file_kw->get_offset(); rd_data_type data_type = file_kw->get_data_type(); diff --git a/lib/resdata/rd_smspec.cpp b/lib/resdata/rd_smspec.cpp index d4db1e099..e39e6278b 100644 --- a/lib/resdata/rd_smspec.cpp +++ b/lib/resdata/rd_smspec.cpp @@ -653,6 +653,9 @@ static void rd_smspec_load_restart(rd_smspec_type *rd_smspec, if (!rd_file_has_kw(header, RESTART_KW)) return; const rd_kw_type *restart_kw = rd_file_iget_named_kw(header, RESTART_KW, 0); + if (restart_kw == nullptr) + throw std::invalid_argument( + "RESTART keyword lookup failed despite keyword presence"); int num_blocks = rd_kw_get_size(restart_kw); num_blocks = (num_blocks < 0) ? 0 : num_blocks; auto tmp_base = rd::checked_calloc(8 * num_blocks + 1); @@ -871,10 +874,24 @@ static bool rd_smspec_fread_header(rd_smspec_type *rd_smspec, int params_index; rd_smspec->num_regions = 0; - rd_smspec->params_size = rd_kw_get_size(keywords); + + if (wells == NULL) + throw std::invalid_argument( + "Could not locate WGNAMES/NAMES keyword in header"); + if (keywords == NULL) + throw std::invalid_argument( + "Could not locate KEYWORDS keyword in header"); if (startdat == NULL) throw std::invalid_argument( "Could not locate STARTDAT keyword in header"); + if (units == NULL) + throw std::invalid_argument( + "Could not locate UNITS keyword in header"); + if (dimens == NULL) + throw std::invalid_argument( + "Could not locate DIMENS keyword in header"); + + rd_smspec->params_size = rd_kw_get_size(keywords); if (rd_file_has_kw(header.get(), NUMS_KW)) nums = rd_file_iget_named_kw(header.get(), NUMS_KW, 0); @@ -882,6 +899,9 @@ static bool rd_smspec_fread_header(rd_smspec_type *rd_smspec, if (rd_file_has_kw(header.get(), INTEHEAD_KW)) { const rd_kw_type *intehead = rd_file_iget_named_kw(header.get(), INTEHEAD_KW, 0); + if (intehead == NULL) + throw std::invalid_argument( + "INTEHEAD keyword lookup failed despite keyword presence"); rd_smspec->unit_system = (ert_rd_unit_enum)rd_kw_iget_int( intehead, INTEHEAD_SMSPEC_UNIT_INDEX); /* @@ -903,12 +923,16 @@ static bool rd_smspec_fread_header(rd_smspec_type *rd_smspec, numlx = rd_file_iget_named_kw(header.get(), NUMLX_KW, 0); numly = rd_file_iget_named_kw(header.get(), NUMLY_KW, 0); numlz = rd_file_iget_named_kw(header.get(), NUMLZ_KW, 0); + rd_smspec->has_lgr = true; } else rd_smspec->has_lgr = false; { int *date = rd_kw_get_int_ptr(startdat); + if (date == NULL) + throw std::invalid_argument( + "STARTDAT keyword has no integer data payload"); int year = date[STARTDAT_YEAR_INDEX]; int month = date[STARTDAT_MONTH_INDEX]; int day = date[STARTDAT_DAY_INDEX]; @@ -1007,6 +1031,9 @@ rd_smspec_type *rd_smspec_fread_alloc(const std::string &header_file, rd_smspec_get_var_node(rd_smspec->misc_var_index, "TIME"); if (time_node) { const char *time_unit = time_node->get_unit(); + if (time_unit == nullptr) + throw std::invalid_argument( + "TIME variable is missing a unit string"); rd_smspec->time_index = time_node->get_params_index(); if (util_string_equal(time_unit, "DAYS")) diff --git a/lib/resdata/rd_unsmry_loader.cpp b/lib/resdata/rd_unsmry_loader.cpp index b26e411e3..1fe402b43 100644 --- a/lib/resdata/rd_unsmry_loader.cpp +++ b/lib/resdata/rd_unsmry_loader.cpp @@ -7,6 +7,9 @@ #include #include +#include +#include +#include #include "detail/resdata/rd_unsmry_loader.hpp" @@ -36,6 +39,20 @@ unsmry_loader::unsmry_loader(const rd_smspec_type *smspec, rd_smspec_get_date_year_index(smspec)}}; rd_file_view_type *file_view = rd_file_get_global_view(file.get()); int length = rd_file_view_get_num_named_kw(file_view, PARAMS_KW); + + if (length > 0) { + const rd_kw_type *params_kw = + rd_file_view_iget_named_kw(file_view, PARAMS_KW, 0); + if (params_kw == nullptr) + throw std::invalid_argument( + "Malformed summary file: missing PARAMS keyword entry"); + + const rd_data_type params_data_type = rd_kw_get_data_type(params_kw); + if (!rd_type_is_float(params_data_type)) + throw std::invalid_argument( + "Malformed summary file: PARAMS keyword is not float"); + } + this->file = file.release(); this->file_view = file_view; this->m_length = length; @@ -53,13 +70,12 @@ std::vector unsmry_loader::get_vector(int pos) const { std::vector data(this->length()); auto index_map = make_int_vector(1, pos); - char buffer[4]; + float value; for (int index = 0; index < this->length(); index++) { rd_file_view_index_fload_kw(file_view, PARAMS_KW, index, - index_map.get(), buffer); - float *data_value = (float *)buffer; - data[index] = *data_value; + index_map.get(), (char *)&value); + data[index] = value; } if (rd_file_view_flags_set(file_view, RD_FILE_CLOSE_STREAM)) diff --git a/lib/resdata/smspec_node.cpp b/lib/resdata/smspec_node.cpp index f5ba942ea..0d74e676a 100644 --- a/lib/resdata/smspec_node.cpp +++ b/lib/resdata/smspec_node.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -8,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -570,12 +572,59 @@ void smspec_node::set_num(const int *grid_dims, int num_) { this->num = num_; if ((var_type == RD_SMSPEC_COMPLETION_VAR) || (var_type == RD_SMSPEC_BLOCK_VAR)) { - int global_index = this->num - 1; - this->ijk[2] = global_index / (grid_dims[0] * grid_dims[1]); - global_index -= this->ijk[2] * (grid_dims[0] * grid_dims[1]); - this->ijk[1] = global_index / grid_dims[0]; - global_index -= this->ijk[1] * grid_dims[0]; - this->ijk[0] = global_index; + if (grid_dims == nullptr) + throw std::invalid_argument( + "Grid dimensions are required for block and completion " + "summary variables"); + + if (grid_dims[0] <= 0 || grid_dims[1] <= 0 || grid_dims[2] <= 0) + throw std::invalid_argument(rd::util::string_format( + "Invalid grid dimensions for summary variable: nx=%d, ny=%d, " + "nz=%d", + grid_dims[0], grid_dims[1], grid_dims[2])); + + const int64_t nx = grid_dims[0]; + const int64_t ny = grid_dims[1]; + const int64_t nz = grid_dims[2]; + + if (nx > std::numeric_limits::max() / ny) + throw std::invalid_argument(rd::util::string_format( + "Grid dimensions overflow for summary variable: nx=%d, ny=%d, " + "nz=%d", + grid_dims[0], grid_dims[1], grid_dims[2])); + + const int64_t plane_size = nx * ny; + if (plane_size <= 0) + throw std::invalid_argument(rd::util::string_format( + "Invalid grid dimensions for summary variable: nx=%d, ny=%d, " + "nz=%d", + grid_dims[0], grid_dims[1], grid_dims[2])); + + if (plane_size > std::numeric_limits::max() / nz) + throw std::invalid_argument(rd::util::string_format( + "Grid dimensions overflow for summary variable: nx=%d, ny=%d, " + "nz=%d", + grid_dims[0], grid_dims[1], grid_dims[2])); + + const int64_t grid_size = plane_size * nz; + if (grid_size <= 0) + throw std::invalid_argument(rd::util::string_format( + "Invalid grid dimensions for summary variable: nx=%d, ny=%d, " + "nz=%d", + grid_dims[0], grid_dims[1], grid_dims[2])); + + int64_t global_index = static_cast(this->num) - 1; + if (global_index < 0 || global_index >= grid_size) + throw std::invalid_argument(rd::util::string_format( + "Invalid NUMS value for summary variable: nums=%d, expected " + "1..%lld", + this->num, static_cast(grid_size))); + + this->ijk[2] = static_cast(global_index / plane_size); + global_index -= static_cast(this->ijk[2]) * plane_size; + this->ijk[1] = static_cast(global_index / nx); + global_index -= static_cast(this->ijk[1]) * nx; + this->ijk[0] = static_cast(global_index); this->ijk[0] += 1; this->ijk[1] += 1; diff --git a/lib/tests/test_rd_kw.cpp b/lib/tests/test_rd_kw.cpp index 8b93dd5dd..154593e83 100644 --- a/lib/tests/test_rd_kw.cpp +++ b/lib/tests/test_rd_kw.cpp @@ -112,6 +112,37 @@ TEST_CASE("rd_kw_iset_string_ptr validates type and length", "[rd_kw]") { } } +TEST_CASE("rd_kw_iget_stripped_string handles width edge cases", "[rd_kw]") { + SECTION("RD_CHAR values can fill the full field width") { + auto char_kw = make_rd_kw("KW", 2, RD_CHAR); + + rd_kw_iset_char_ptr(char_kw.get(), 0, "FOPRTEST"); + rd_kw_iset_char_ptr(char_kw.get(), 1, "BPR"); + + REQUIRE(rd_kw_iget_stripped_string(char_kw.get(), 0) == "FOPRTEST"); + REQUIRE(rd_kw_iget_stripped_string(char_kw.get(), 1) == "BPR"); + } + + SECTION("RD_STRING values can fill the declared field width") { + auto string_kw = make_rd_kw("KW", 1, RD_STRING(12)); + + rd_kw_iset_string_ptr(string_kw.get(), 0, "0123456789AB"); + + REQUIRE(rd_kw_iget_stripped_string(string_kw.get(), 0) == + "0123456789AB"); + } + + SECTION("embedded NUL stops the extracted string before field width") { + auto string_kw = make_rd_kw("KW", 1, RD_STRING(12)); + char *raw = static_cast(rd_kw_iget_ptr(string_kw.get(), 0)); + + std::memcpy(raw, "ABCD\0EFGHIJK", 12); + raw[12] = '\0'; + + REQUIRE(rd_kw_iget_stripped_string(string_kw.get(), 0) == "ABCD"); + } +} + TEST_CASE("scalar_set/scale/shift validate the type", "[rd_kw]") { auto float_kw = make_rd_kw("KW", 3, RD_FLOAT); REQUIRE_THROWS_WITH(rd_kw_scalar_set_int(float_kw.get(), 1), diff --git a/tests/rd_tests/test_rd_sum.py b/tests/rd_tests/test_rd_sum.py index 5a6756f22..9d2a34086 100644 --- a/tests/rd_tests/test_rd_sum.py +++ b/tests/rd_tests/test_rd_sum.py @@ -1524,3 +1524,99 @@ def test_get_general_var_index_with_unknown_key_raises(): with pytest.raises(IndexError, match="Invalid lookup summary object"): summary.get_general_var_index("NO_SUCH_KEY") + + +@pytest.mark.usefixtures("use_tmpdir") +def test_smspec_keyword_without_nul_terminator_is_read_correctly(): + smspec_kws = [ + ("INTEHEAD", np.array([1, 100], dtype=np.int32)), + ("RESTART ", [" "] * 9), + ("DIMENS ", np.array([2, 10, 10, 10, 0, 0], dtype=np.int32)), + ("KEYWORDS", ["TIME ", "FOPRTEST"]), + ("WGNAMES ", [":+:+:+:+", ":+:+:+:+"]), + ("NUMS ", np.array([-32676, 0], dtype=np.int32)), + ("UNITS ", ["DAYS ", "SM3 "]), + ("STARTDAT", np.array([1, 1, 2000, 0, 0, 0], dtype=np.int32)), + ] + resfo.write("TEST.SMSPEC", smspec_kws) + + unsmry_kws = [ + ("SEQHDR ", np.array([1], dtype=np.int32)), + ("MINISTEP", np.array([0], dtype=np.int32)), + ("PARAMS ", np.array([1.0, 100.0], dtype=np.float32)), + ] + resfo.write("TEST.UNSMRY", unsmry_kws) + + summary = Summary("TEST") + assert summary.has_key("FOPRTEST") + + +@pytest.mark.usefixtures("use_tmpdir") +def test_smspec_missing_required_keyword_raises(): + for missing_kw in ("WGNAMES ", "KEYWORDS", "UNITS ", "DIMENS "): + smspec_kws = [ + ("INTEHEAD", np.array([1, 100], dtype=np.int32)), + ("RESTART ", [" "] * 9), + ("DIMENS ", np.array([2, 10, 10, 10, 0, 0], dtype=np.int32)), + ("KEYWORDS", ["TIME ", "FOPR "]), + ("WGNAMES ", [":+:+:+:+", ":+:+:+:+"]), + ("NUMS ", np.array([-32676, 0], dtype=np.int32)), + ("UNITS ", ["DAYS ", "SM3 "]), + ("STARTDAT", np.array([1, 1, 2000, 0, 0, 0], dtype=np.int32)), + ] + smspec_kws = [(k, v) for k, v in smspec_kws if k != missing_kw] + resfo.write("TEST.SMSPEC", smspec_kws) + resfo.write( + "TEST.UNSMRY", + [ + ("SEQHDR ", np.array([1], dtype=np.int32)), + ("MINISTEP", np.array([0], dtype=np.int32)), + ("PARAMS ", np.array([1.0, 100.0], dtype=np.float32)), + ], + ) + + with pytest.raises((IOError, ValueError)): + Summary("TEST") + + +@pytest.mark.usefixtures("use_tmpdir") +def test_smspec_startdat_with_empty_payload_raises(): + smspec_kws = [ + ("INTEHEAD", np.array([1, 100], dtype=np.int32)), + ("RESTART ", [" "] * 9), + ("DIMENS ", np.array([2, 10, 10, 10, 0, 0], dtype=np.int32)), + ("KEYWORDS", ["TIME ", "FOPR "]), + ("WGNAMES ", [":+:+:+:+", ":+:+:+:+"]), + ("NUMS ", np.array([-32676, 0], dtype=np.int32)), + ("UNITS ", ["DAYS ", "SM3 "]), + ("STARTDAT", ["NOTANINT"]), + ] + resfo.write("TEST.SMSPEC", smspec_kws) + resfo.write( + "TEST.UNSMRY", + [ + ("SEQHDR ", np.array([1], dtype=np.int32)), + ("MINISTEP", np.array([0], dtype=np.int32)), + ("PARAMS ", np.array([1.0, 100.0], dtype=np.float32)), + ], + ) + + with pytest.raises((IOError, ValueError)): + Summary("TEST") + + +@pytest.mark.usefixtures("use_tmpdir") +def test_unsmry_params_non_float_raises(): + create_summary(summary_keys=("FOPR",), times=(0.0, 1.0, 2.0)) + + kws = list(resfo.read("TEST.UNSMRY")) + patched = [] + for name, value in kws: + if name.strip() == "PARAMS": + patched.append((name, value.astype(np.int32))) + else: + patched.append((name, value)) + resfo.write("TEST.UNSMRY", patched) + + with pytest.raises((IOError, ValueError)): + Summary("TEST")