Opened 6 years ago

Closed 5 years ago

#862 closed patch (fixed)

WKBReader::readHEX: Incorrect use of char_traits<char>::eof()

Reported by: goatbar Owned by: geos-devel@…
Priority: major Milestone: 3.6.4
Component: Default Version: 3.6.2
Severity: Unassigned Keywords:
Cc:

Description

char_traits<char>::eof() returns an int. It doesn't always work to compare to a char.

I was seeing invalid 255s thrown.

My sample fix... I add localized vars, added consts, and factored out the switch statement so it's only in there 1 time.

  • geos/src/io/WKBReader.cpp

    a b  
    7474        return os;
    7575}
    7676
     77namespace {
     78
     79unsigned char HexToUChar(int val)
     80{
     81        switch ( val )
     82        {
     83                case '0' :
     84                        return 0;
     85                case '1' :
     86                        return 1;
     87                case '2' :
     88                        return 2;
     89                case '3' :
     90                        return 3;
     91                case '4' :
     92                        return 4;
     93                case '5' :
     94                        return 5;
     95                case '6' :
     96                        return 6;
     97                case '7' :
     98                        return 7;
     99                case '8' :
     100                        return 8;
     101                case '9' :
     102                        return 9;
     103                case 'A' :
     104                case 'a' :
     105                        return 10;
     106                case 'B' :
     107                case 'b' :
     108                        return 11;
     109                case 'C' :
     110                case 'c' :
     111                        return 12;
     112                case 'D' :
     113                case 'd' :
     114                        return 13;
     115                case 'E' :
     116                case 'e' :
     117                        return 14;
     118                case 'F' :
     119                case 'f' :
     120                        return 15;
     121                default:
     122                        throw ParseException("Invalid HEX char");
     123        }
     124}
     125
     126}  // namespace
    77127
    78128Geometry *
    79129WKBReader::readHEX(istream &is)
    … …  
    81131        // setup input/output stream
    82132        stringstream os(ios_base::binary|ios_base::in|ios_base::out);
    83133
    84         unsigned char result_high, result_low, value;
    85         char high, low;
    86 
    87         while( (high = static_cast<char>(is.get())) != char_traits<char>::eof() )
     134        while( true )
    88135        {
    89                 // geth the low part of the byte
    90                 low = static_cast<char>(is.get());
    91                 if ( low == char_traits<char>::eof() )
    92                   throw ParseException("Premature end of HEX string");
    93 
    94                 switch (high)
    95                 {
    96                         case '0' :
    97                                 result_high = 0;
    98                                 break;
    99                         case '1' :
    100                                 result_high = 1;
    101                                 break;
    102                         case '2' :
    103                                 result_high = 2;
    104                                 break;
    105                         case '3' :
    106                                 result_high = 3;
    107                                 break;
    108                         case '4' :
    109                                 result_high = 4;
    110                                 break;
    111                         case '5' :
    112                                 result_high = 5;
    113                                 break;
    114                         case '6' :
    115                                 result_high = 6;
    116                                 break;
    117                         case '7' :
    118                                 result_high = 7;
    119                                 break;
    120                         case '8' :
    121                                 result_high = 8;
    122                                 break;
    123                         case '9' :
    124                                 result_high = 9;
    125                                 break;
    126                         case 'A' :
    127             case 'a' :
    128                                 result_high = 10;
    129                                 break;
    130                         case 'B' :
    131             case 'b' :
    132                                 result_high = 11;
    133                                 break;
    134                         case 'C' :
    135                         case 'c' :
    136                                 result_high = 12;
    137                                 break;
    138                         case 'D' :
    139                         case 'd' :
    140                                 result_high = 13;
    141                                 break;
    142                         case 'E' :
    143                         case 'e' :
    144                                 result_high = 14;
    145                                 break;
    146                         case 'F' :
    147                         case 'f' :
    148                                 result_high = 15;
    149                                 break;
    150                         default:
    151                                 throw  ParseException("Invalid HEX char");
    152                 }
    153 
    154                 switch (low)
    155                 {
    156                         case '0' :
    157                                 result_low = 0;
    158                                 break;
    159                         case '1' :
    160                                 result_low = 1;
    161                                 break;
    162                         case '2' :
    163                                 result_low = 2;
    164                                 break;
    165                         case '3' :
    166                                 result_low = 3;
    167                                 break;
    168                         case '4' :
    169                                 result_low = 4;
    170                                 break;
    171                         case '5' :
    172                                 result_low = 5;
    173                                 break;
    174                         case '6' :
    175                                 result_low = 6;
    176                                 break;
    177                         case '7' :
    178                                 result_low = 7;
    179                                 break;
    180                         case '8' :
    181                                 result_low = 8;
    182                                 break;
    183                         case '9' :
    184                                 result_low = 9;
    185                                 break;
    186                         case 'A' :
    187             case 'a' :
    188                                 result_low = 10;
    189                                 break;
    190                         case 'B' :
    191             case 'b' :
    192                                 result_low = 11;
    193                                 break;
    194                         case 'C' :
    195                         case 'c' :
    196                                 result_low = 12;
    197                                 break;
    198                         case 'D' :
    199                         case 'd' :
    200                                 result_low = 13;
    201                                 break;
    202                         case 'E' :
    203                         case 'e' :
    204                                 result_low = 14;
    205                                 break;
    206                         case 'F' :
    207                         case 'f' :
    208                                 result_low = 15;
    209                                 break;
    210                         default:
    211                                 throw  ParseException("Invalid HEX char");
    212                 }
     136                const int input_high = is.get();
     137                if (input_high == char_traits<char>::eof())
     138                        break;
     139
     140                const int input_low = is.get();
     141                if (input_low == char_traits<char>::eof())
     142                        throw ParseException("Premature end of HEX string");
     143
     144                const char high = static_cast<char>(input_high);
     145                const char low = static_cast<char>(input_low);
     146
     147                const unsigned char result_high = HexToUChar(high);
     148                const unsigned char result_low = HexToUChar(low);
    213149
    214                 value = static_cast<char>((result_high<<4) + result_low);
     150                const unsigned char value =
     151                        static_cast<char>((result_high<<4) + result_low);
    215152
    216153#if DEBUG_HEX_READER
    217154        cout<<"HEX "<<high<<low<<" -> DEC "<<(int)value<<endl;

Change History (3)

comment:2 by robe, 6 years ago

Milestone: 3.6.3 → 3.6.4
Type: defect → patch

comment:3 by dbaston, 5 years ago

Resolution: → fixed
Status: new → closed

Committed in 375f1f15fe38b8ad14001efbcb0ba89a76937025/git

Note: See TracTickets for help on using tickets.