ECCC-MSC / libecbufr

libecbufr is a general purpose, template-oriented BUFR encoding/decoding library
Other
10 stars 7 forks source link

Need error checking on BUFR Master Table number #23

Open vsouvan opened 4 years ago

vsouvan commented 4 years ago

Background

In a BUFR message, Octet 4 of Section 1 contains the BUFR Master Table number. This number is an overarching identifier for the content of Tables B and D, and NOT a version number. Currently, the BUFR regulations permit two legal values for this: zero and ten. (For Meteorology and Oceanography, respectively.) Libecbufr only handles the Meteorology Tables. The Oceanography Tables find little operational use, at least for the time being, and are not implemented by CMC.

Bug Description

Therefore, for libecbufr to proceed with decoding when the Master Table number is a non-zero value is not correct behaviour.

Proposed Fix

The software should check Octet 4 and proceed as follows:

Value = 0 ; proceed with decoding

Value = 10; issue message "Oceanography Tables are not supported by libecbufr" or in French "Les tables océanographiques ne sont pas prises en charge par libecbufr". Stop processing.

Any other value: Output the value; issue message "This Master Table number is not supported by WMO BUFR regulations. Check Octet 4 in Section 1." or in French "Ce numéro de table maîtresse n'est pas pris en charge par les règles d'encodage BUFR de l'OMM. Vérifiez l'octet 4 de la section 1." Stop processing.


Imported from Launchpad using lp2gh.

vsouvan commented 4 years ago

(by chris-beauregard) "Stop processing."

Why not just set BUFR_FLAG_INVALID and proceeding like we normally do? Refusing to process at all completely precludes using LibECBUFR to even attempt to handle the other master tables, if only for development purposes.

vsouvan commented 4 years ago

(by yves-pelletier) If one day we get to the point where Master Table 10 is implemented operationally, we can change the behavior. Currently, Master Table 10 only makes sense in development mode. We don't even have copies of the Tables for Master Table 10, let alone distribute them with the software.

For any other number than 0 and 10, we could choose to assume that the sender just made a mistake and meant zero. But that's a big leap of faith, one I am not prepared to make in an operational context. But yes, we should have the option to force decoding (with the assumption of Master Table 0) in investigation or development mode.

vsouvan commented 4 years ago

(by chris-beauregard) Yeah, I'm working under the assumption that operational systems won't typically be ignoring BUFR_FLAG_INVALID. If they are, they've got bigger problems than the decoder making assumptions about octet 4.

vsouvan commented 4 years ago

(by chris-beauregard) The latest sets of changes completely broke regression tests. And given that one of the broken tests is test_zero (which uses the API to encode and then immediately decode a message), I'm thinking it's not entirely a problem with the regression data.

vsouvan commented 4 years ago

(by vanh-souvanlasy) the pre decode checking was wrong, tests should be ok now with commit rev.305

vsouvan commented 4 years ago

(by chris-beauregard) That's got it. Thanks Vanh.