From 6a5749ecb7c237458a89c6155842a767e7e8602c Mon Sep 17 00:00:00 2001 From: Shahram Najm Date: Wed, 31 Jul 2019 18:52:53 +0100 Subject: [PATCH] ECC-428: Inconsistent number of values when decoding compressed BUFR data --- src/grib_accessor_class_bufr_data_array.c | 182 +++++++++++----------- src/grib_api_internal.h | 1 + src/grib_context.c | 11 +- tests/CMakeLists.txt | 1 + tests/bufr_ecc-428.sh | 56 +++++++ 5 files changed, 161 insertions(+), 90 deletions(-) create mode 100755 tests/bufr_ecc-428.sh diff --git a/src/grib_accessor_class_bufr_data_array.c b/src/grib_accessor_class_bufr_data_array.c index 29d2c5f9e..3d4175371 100644 --- a/src/grib_accessor_class_bufr_data_array.c +++ b/src/grib_accessor_class_bufr_data_array.c @@ -559,33 +559,33 @@ static int decode_string_array(grib_context* c, unsigned char* data, long* pos, sval=(char*)grib_context_malloc_clear(c,modifiedWidth/8+1); CHECK_END_DATA_RETURN(c, self, modifiedWidth, *err); if (*err) { - grib_sarray_push(c,sa,sval); - grib_vsarray_push(c,self->stringValues,sa); - return ret; + grib_sarray_push(c,sa,sval); + grib_vsarray_push(c,self->stringValues,sa); + return ret; } grib_decode_string(data,pos,modifiedWidth/8,sval); CHECK_END_DATA_RETURN(c, self, 6, *err); if (*err) { - grib_sarray_push(c,sa,sval); - grib_vsarray_push(c,self->stringValues,sa); - return ret; + grib_sarray_push(c,sa,sval); + grib_vsarray_push(c,self->stringValues,sa); + return ret; } width=grib_decode_unsigned_long(data,pos,6); if (width) { CHECK_END_DATA_RETURN(c, self, width*8*self->numberOfSubsets, *err); - if (*err) { - grib_sarray_push(c,sa,sval); - grib_vsarray_push(c,self->stringValues,sa); - return ret; - } - grib_context_free(c,sval); - for (j=0;jnumberOfSubsets;j++) { - sval=(char*)grib_context_malloc_clear(c,width+1); - grib_decode_string(data,pos,width,sval); - grib_sarray_push(c,sa,sval); - } + if (*err) { + grib_sarray_push(c,sa,sval); + grib_vsarray_push(c,self->stringValues,sa); + return ret; + } + grib_context_free(c,sval); + for (j=0;jnumberOfSubsets;j++) { + sval=(char*)grib_context_malloc_clear(c,width+1); + grib_decode_string(data,pos,width,sval); + grib_sarray_push(c,sa,sval); + } } else { - grib_sarray_push(c,sa,sval); + grib_sarray_push(c,sa,sval); } grib_vsarray_push(c,self->stringValues,sa); return ret; @@ -609,13 +609,13 @@ static grib_darray* decode_double_array(grib_context* c,unsigned char* data,long CHECK_END_DATA_RETURN(c, self, modifiedWidth+6, NULL); if (*err) { - dval=GRIB_MISSING_DOUBLE; - lval=0; - grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g", modifiedWidth,lval,dval); - ret=grib_darray_new(c,DYN_ARRAY_SIZE_INIT,DYN_ARRAY_SIZE_INCR); - grib_darray_push(c,ret,dval); - *err=0; - return ret; + dval=GRIB_MISSING_DOUBLE; + lval=0; + grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g", modifiedWidth,lval,dval); + ret=grib_darray_new(c,DYN_ARRAY_SIZE_INIT,DYN_ARRAY_SIZE_INCR); + grib_darray_push(c,ret,dval); + *err=0; + return ret; } lval=grib_decode_unsigned_long(data,pos,modifiedWidth); localReference=(long)lval+modifiedReference; @@ -625,13 +625,13 @@ static grib_darray* decode_double_array(grib_context* c,unsigned char* data,long if (localWidth) { CHECK_END_DATA_RETURN(c, self, localWidth*self->numberOfSubsets, NULL); if (*err) { - dval=GRIB_MISSING_DOUBLE; - lval=0; - grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g", modifiedWidth,lval,dval); - ret=grib_darray_new(c,DYN_ARRAY_SIZE_INIT,DYN_ARRAY_SIZE_INCR); - grib_darray_push(c,ret,dval); - *err=0; - return ret; + dval=GRIB_MISSING_DOUBLE; + lval=0; + grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g", modifiedWidth,lval,dval); + ret=grib_darray_new(c,DYN_ARRAY_SIZE_INIT,DYN_ARRAY_SIZE_INCR); + grib_darray_push(c,ret,dval); + *err=0; + return ret; } for (j=0;jnumberOfSubsets;j++) { lval=grib_decode_unsigned_long(data,pos,localWidth); @@ -643,13 +643,21 @@ static grib_darray* decode_double_array(grib_context* c,unsigned char* data,long grib_darray_push(c,ret,dval); } } else { + // ECC-428 if (grib_is_all_bits_one(lval,modifiedWidth) && canBeMissing) { dval=GRIB_MISSING_DOUBLE; } else { dval=localReference*modifiedFactor; } - grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g", modifiedWidth,lval,dval); - grib_darray_push(c,ret,dval); + if(c->bufr_multi_element_constant_arrays) { + grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g (const array multi values)", modifiedWidth,lval,dval,bd->code); + for (j=0;jnumberOfSubsets;j++) { + grib_darray_push(c,ret,dval); + } + } else { + grib_context_log(c, GRIB_LOG_DEBUG," modifiedWidth=%ld lval=%ld dval=%g (const array single value)", modifiedWidth,lval,dval,bd->code); + grib_darray_push(c,ret,dval); + } } return ret; @@ -671,10 +679,10 @@ static int encode_string_array(grib_context* c,grib_buffer* buff,long* pos, bufr if (n<=0) return GRIB_NO_VALUES; if (grib_sarray_used_size(stringValues)==1) { - n=1; - ival=0; + n=1; + ival=0; } else { - ival=self->iss_list->v[0]; + ival=self->iss_list->v[0]; } if (n>grib_sarray_used_size(stringValues)) @@ -708,7 +716,7 @@ static void set_missing_long_to_double(grib_darray* dvalues) /* ECC-750: The 'factor' argument is 10^-scale */ static int descriptor_get_min_max(bufr_descriptor* bd, long width, long reference, double factor, - double* minAllowed, double* maxAllowed) + double* minAllowed, double* maxAllowed) { /* Maximum value is allowed to be the largest number (all bits 1) which means it's MISSING */ unsigned long max1 = (1UL << width) - 1; /* Highest value for number with 'width' bits */ @@ -768,11 +776,11 @@ static int encode_double_array(grib_context* c,grib_buffer* buff,long* pos, bufr if (*v > maxAllowed || *v < minAllowed) { if (dont_fail_if_out_of_range) { grib_context_log(c, GRIB_LOG_ERROR, "encode_double_array: %s. Value (%g) out of range (minAllowed=%g, maxAllowed=%g)." - " Setting it to missing value\n", bd->shortName, *v, minAllowed, maxAllowed); + " Setting it to missing value\n", bd->shortName, *v, minAllowed, maxAllowed); grib_set_bits_on(buff->data,pos,modifiedWidth); } else { grib_context_log(c, GRIB_LOG_ERROR, "encode_double_array: %s. Value (%g) out of range (minAllowed=%g, maxAllowed=%g).", - bd->shortName, *v, minAllowed, maxAllowed); + bd->shortName, *v, minAllowed, maxAllowed); return GRIB_OUT_OF_RANGE; /* ECC-611 */ } } else { @@ -790,8 +798,8 @@ static int encode_double_array(grib_context* c,grib_buffer* buff,long* pos, bufr val0=dvalues->v[self->iss_list->v[0]]; is_constant=1; for (i=0;iv[self->iss_list->v[i]]; - if (val0 != values[i]) is_constant=0; + values[i]=dvalues->v[self->iss_list->v[i]]; + if (val0 != values[i]) is_constant=0; } v=values; @@ -822,8 +830,8 @@ static int encode_double_array(grib_context* c,grib_buffer* buff,long* pos, bufr /* Turn out-of-range values into 'missing' */ if (*v!=GRIB_MISSING_DOUBLE && (*v < minAllowed || *v > maxAllowed)) { grib_context_log(c, GRIB_LOG_ERROR, "encode_double_array: %s. Value at index %ld (%g) out of range (minAllowed=%g, maxAllowed=%g)." - " Setting it to missing value\n", - bd->shortName, (long)ii, *v, minAllowed, maxAllowed); + " Setting it to missing value\n", + bd->shortName, (long)ii, *v, minAllowed, maxAllowed); *v = GRIB_MISSING_DOUBLE; } ii++; @@ -850,12 +858,12 @@ static int encode_double_array(grib_context* c,grib_buffer* buff,long* pos, bufr } if (max>maxAllowed && max!=GRIB_MISSING_DOUBLE) { grib_context_log(c, GRIB_LOG_ERROR, "encode_double_array: %s. Maximum value (value[%lu]=%g) out of range (maxAllowed=%g).", - bd->shortName, index_of_max, max, maxAllowed, index_of_max); + bd->shortName, index_of_max, max, maxAllowed, index_of_max); return GRIB_OUT_OF_RANGE; } if (minshortName, index_of_min, min, minAllowed); + bd->shortName, index_of_min, min, minAllowed); return GRIB_OUT_OF_RANGE; } @@ -931,13 +939,13 @@ static int encode_double_value(grib_context* c,grib_buffer* buff,long* pos,bufr_ else if (value>maxAllowed || valueshortName, value, minAllowed, maxAllowed); + " Setting it to missing value\n", + bd->shortName, value, minAllowed, maxAllowed); value = GRIB_MISSING_DOUBLE; /* Ignore the bad value and instead use 'missing' */ grib_set_bits_on(buff->data,pos,modifiedWidth); } else { grib_context_log(c, GRIB_LOG_ERROR, "encode_double_value: %s. Value (%g) out of range (minAllowed=%g, maxAllowed=%g).", - bd->shortName, value, minAllowed, maxAllowed); + bd->shortName, value, minAllowed, maxAllowed); return GRIB_OUT_OF_RANGE; } } @@ -1109,28 +1117,28 @@ static int decode_replication(grib_context* c,grib_accessor_bufr_data_array* sel grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication localReference width=%ld", descriptors[i]->width); CHECK_END_DATA_RETURN(c, self, descriptors[i]->width+6, *err); if (*err) { - *numberOfRepetitions=0; + *numberOfRepetitions=0; } else { - localReference=grib_decode_unsigned_long(data,pos,descriptors[i]->width)+descriptors[i]->reference; - grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication localWidth width=6"); - width=grib_decode_unsigned_long(data,pos,6); - if (width) { - grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication is NOT constant for compressed data!"); - /* delayed replication number is not constant. NOT IMPLEMENTED */ - return GRIB_NOT_IMPLEMENTED; - } else { - *numberOfRepetitions=localReference*descriptors[i]->factor; - grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication value=%ld",*numberOfRepetitions); - } + localReference=grib_decode_unsigned_long(data,pos,descriptors[i]->width)+descriptors[i]->reference; + grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication localWidth width=6"); + width=grib_decode_unsigned_long(data,pos,6); + if (width) { + grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication is NOT constant for compressed data!"); + /* delayed replication number is not constant. NOT IMPLEMENTED */ + return GRIB_NOT_IMPLEMENTED; + } else { + *numberOfRepetitions=localReference*descriptors[i]->factor; + grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication value=%ld",*numberOfRepetitions); + } } } else { CHECK_END_DATA_RETURN(c, self, descriptors[i]->width, *err); if (*err) { - *numberOfRepetitions=0; + *numberOfRepetitions=0; } else { - *numberOfRepetitions=grib_decode_unsigned_long(data,pos,descriptors[i]->width)+ - descriptors[i]->reference*descriptors[i]->factor; - grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication value=%ld",*numberOfRepetitions); + *numberOfRepetitions=grib_decode_unsigned_long(data,pos,descriptors[i]->width)+ + descriptors[i]->reference*descriptors[i]->factor; + grib_context_log(c, GRIB_LOG_DEBUG,"BUFR data decoding: \tdelayed replication value=%ld",*numberOfRepetitions); } } if (self->compressedData) { @@ -1364,7 +1372,7 @@ static int encode_element(grib_context* c,grib_accessor_bufr_data_array* self,in err=encode_double_value(c,buff,pos,bd,self,self->numericValues->v[subsetIndex]->v[elementIndex]); if (err) { grib_context_log(c,GRIB_LOG_ERROR,"Cannot encode %s=%g (subset=%d)", /*subsetIndex starts from 0*/ - bd->shortName, self->numericValues->v[subsetIndex]->v[elementIndex], subsetIndex+1); + bd->shortName, self->numericValues->v[subsetIndex]->v[elementIndex], subsetIndex+1); } } } @@ -1957,9 +1965,9 @@ static GRIB_INLINE int significanceQualifierIndex(int X,int Y) } static GRIB_INLINE void reset_deeper_qualifiers( - grib_accessor* significanceQualifierGroup[], - const int* const significanceQualifierDepth, - int numElements, int depth) + grib_accessor* significanceQualifierGroup[], + const int* const significanceQualifierDepth, + int numElements, int depth) { int i; for (i=0;iaccessor,"index"); - grib_unpack_long(anindex,index,&l); + long index[1]; + grib_accessor* anindex=grib_accessor_get_attribute(al->accessor,"index"); + grib_unpack_long(anindex,index,&l); #endif - return 1; - } + return 1; + } } return 0; } @@ -2132,7 +2140,7 @@ static void print_bitmap_debug_info(grib_context* c, bitmap_s* bitmap, grib_acce } static int bitmap_init(grib_context* c, bitmap_s* bitmap, - grib_accessors_list* bitmapStart, int bitmapSize, grib_accessors_list* lastAccessorInList) + grib_accessors_list* bitmapStart, int bitmapSize, grib_accessors_list* lastAccessorInList) { int ret=0,i; bitmap->cursor=bitmapStart->next; @@ -2305,7 +2313,7 @@ static int create_keys(grib_accessor* a,long onlySubset,long startSubset,long en } elementFromBitmap=NULL; if (descriptor->F==0 && IS_QUALIFIER(descriptor->X) && - self->unpackMode==CODES_BUFR_UNPACK_STRUCTURE) { + self->unpackMode==CODES_BUFR_UNPACK_STRUCTURE) { int sidx=significanceQualifierIndex(descriptor->X,descriptor->Y); groupNumber++; @@ -2315,7 +2323,7 @@ static int create_keys(grib_accessor* a,long onlySubset,long startSubset,long en if (depth < max_depth) { /* If depth >= max_depth, then no entry will be deeper so no need for call */ reset_deeper_qualifiers(significanceQualifierGroup,significanceQualifierDepth, - number_of_qualifiers,depth); + number_of_qualifiers,depth); } } else { /* if (forceGroupClosure) { */ @@ -2362,7 +2370,7 @@ static int create_keys(grib_accessor* a,long onlySubset,long startSubset,long en groupSection=bitmapGroup[bitmapIndex]->parent; depth=bitmapDepth[bitmapIndex]; reset_deeper_qualifiers(significanceQualifierGroup,significanceQualifierDepth, - number_of_qualifiers,depth); + number_of_qualifiers,depth); /* TODO: This branch is not reached in our tests! */ reset_deeper_qualifiers(bitmapGroup,bitmapDepth,MAX_NUMBER_OF_BITMAPS,depth); } else { @@ -2470,7 +2478,7 @@ static int create_keys(grib_accessor* a,long onlySubset,long startSubset,long en case 31021: associatedFieldSignificanceAccessor=elementAccessor; break; - /*case 33007:*/ + /*case 33007:*/ /* ECC-690: See later */ /* break; */ default: @@ -2553,7 +2561,7 @@ static int set_to_missing_if_out_of_range(grib_handle* h) /* First check if the transient key is set */ long setToMissingIfOutOfRange=0; if (grib_get_long(h, "setToMissingIfOutOfRange", &setToMissingIfOutOfRange)==GRIB_SUCCESS && - setToMissingIfOutOfRange != 0) + setToMissingIfOutOfRange != 0) { return 1; } @@ -2767,10 +2775,10 @@ static int process_elements(grib_accessor* a,int flag,long onlySubset,long start while (ip>=0 && n[ip]==0) { nn[ip]--; if (nn[ip]<=0) { - numberOfNestedRepetitions--; + numberOfNestedRepetitions--; } else { - n[ip]=numberOfElementsToRepeat[ip]; - i=startRepetition[ip]; + n[ip]=numberOfElementsToRepeat[ip]; + i=startRepetition[ip]; } ip--; } diff --git a/src/grib_api_internal.h b/src/grib_api_internal.h index 285a67e8e..023b2b219 100644 --- a/src/grib_api_internal.h +++ b/src/grib_api_internal.h @@ -1053,6 +1053,7 @@ struct grib_context int ieee_packing; int bufrdc_mode; int bufr_set_to_missing_if_out_of_range; + int bufr_multi_element_constant_arrays; FILE* log_stream; grib_trie* classes; grib_trie* lists; diff --git a/src/grib_context.c b/src/grib_context.c index d8c473d75..062a2f582 100644 --- a/src/grib_context.c +++ b/src/grib_context.c @@ -335,6 +335,7 @@ static grib_context default_grib_context = { 0, /* ieee_packing */ 0, /* bufrdc_mode */ 0, /* bufr_set_to_missing_if_out_of_range */ + 0, /* bufr_multi_element_constant_arrays */ 0, /* log_stream */ 0, /* classes */ 0, /* lists */ @@ -368,6 +369,7 @@ grib_context* grib_context_get_default() const char* keep_matrix = NULL; const char* bufrdc_mode = NULL; const char* bufr_set_to_missing_if_out_of_range = NULL; + const char* bufr_multi_element_constant_arrays = NULL; const char* file_pool_max_opened_files = NULL; #ifdef ENABLE_FLOATING_POINT_EXCEPTIONS @@ -375,8 +377,9 @@ grib_context* grib_context_get_default() #endif write_on_fail = codes_getenv("ECCODES_GRIB_WRITE_ON_FAIL"); - bufrdc_mode = codes_getenv("ECCODES_BUFRDC_MODE_ON"); - bufr_set_to_missing_if_out_of_range = codes_getenv("ECCODES_BUFR_SET_TO_MISSING_IF_OUT_OF_RANGE"); + bufrdc_mode = getenv("ECCODES_BUFRDC_MODE_ON"); + bufr_set_to_missing_if_out_of_range = getenv("ECCODES_BUFR_SET_TO_MISSING_IF_OUT_OF_RANGE"); + bufr_multi_element_constant_arrays = getenv("ECCODES_BUFR_MULTI_ELEMENT_CONSTANT_ARRAYS"); large_constant_fields = codes_getenv("ECCODES_GRIB_LARGE_CONSTANT_FIELDS"); no_abort = codes_getenv("ECCODES_NO_ABORT"); debug = codes_getenv("ECCODES_DEBUG"); @@ -387,7 +390,7 @@ grib_context* grib_context_get_default() no_big_group_split = codes_getenv("ECCODES_GRIB_NO_BIG_GROUP_SPLIT"); no_spd = codes_getenv("ECCODES_GRIB_NO_SPD"); keep_matrix = codes_getenv("ECCODES_GRIB_KEEP_MATRIX"); - file_pool_max_opened_files = codes_getenv("ECCODES_FILE_POOL_MAX_OPENED_FILES"); + file_pool_max_opened_files = getenv("ECCODES_FILE_POOL_MAX_OPENED_FILES"); /* On UNIX, when we read from a file we get exactly what is in the file on disk. * But on Windows a file can be opened in binary or text mode. In binary mode the system behaves exactly as in UNIX. @@ -473,6 +476,8 @@ grib_context* grib_context_get_default() default_grib_context.bufrdc_mode = bufrdc_mode ? atoi(bufrdc_mode) : 0; default_grib_context.bufr_set_to_missing_if_out_of_range = bufr_set_to_missing_if_out_of_range ? atoi(bufr_set_to_missing_if_out_of_range) : 0; + default_grib_context.bufr_multi_element_constant_arrays = bufr_multi_element_constant_arrays ? + atoi(bufr_multi_element_constant_arrays) : 0; default_grib_context.file_pool_max_opened_files = file_pool_max_opened_files ? atoi(file_pool_max_opened_files) : DEFAULT_FILE_POOL_MAX_OPENED_FILES; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6accedb9a..dc5c8ffda 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -87,6 +87,7 @@ list( APPEND tests_data_reqd bufr_keys_iter bufr_get_element bufr_wmo_tables + bufr_ecc-428 bufr_ecc-197 bufr_ecc-286 bufr_ecc-288 diff --git a/tests/bufr_ecc-428.sh b/tests/bufr_ecc-428.sh new file mode 100755 index 000000000..ac5b7329d --- /dev/null +++ b/tests/bufr_ecc-428.sh @@ -0,0 +1,56 @@ +#!/bin/sh +# Copyright 2005-2019 ECMWF. +# +# This software is licensed under the terms of the Apache Licence Version 2.0 +# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. +# +# In applying this licence, ECMWF does not waive the privileges and immunities granted to it by +# virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction. +# + +. ./include.sh + +# --------------------------------------------------------- +# This is the test for the JIRA issue ECC-428 +# Decoding compressed BUFR data: +# Option to have constant arrays with multiple repeated values +# rather than a single value +# --------------------------------------------------------- +cd ${data_dir}/bufr +label="bufr_ecc_428_test" + +tempRules=temp.${label}.filter +tempText=temp.${label}.text +tempRef1=temp.${label}.ref1 +tempRef2=temp.${label}.ref2 + +# -------------------------------------------------------- +# Test 1 +# -------------------------------------------------------- +bufrFile=airs_57.bufr # this has 15 subsets +cat > $tempRules < $tempText +echo "784" > $tempRef1 +diff $tempRef1 $tempText + +export ECCODES_BUFR_MULTI_ELEMENT_CONSTANT_ARRAYS=1 +${tools_dir}/codes_bufr_filter $tempRules $bufrFile > $tempText +echo "784 784 784 784 784 784 784 784 784 784 784 784 784 784 784" > $tempRef2 +diff $tempRef2 $tempText + +unset ECCODES_BUFR_MULTI_ELEMENT_CONSTANT_ARRAYS +${tools_dir}/codes_bufr_filter $tempRules $bufrFile > $tempText +diff $tempRef1 $tempText + +export ECCODES_BUFR_MULTI_ELEMENT_CONSTANT_ARRAYS=0 +${tools_dir}/codes_bufr_filter $tempRules $bufrFile > $tempText +diff $tempRef1 $tempText + + + +# ------------------------ +rm -rf $tempRules $tempText $tempRef1 $tempRef2