cbor: ensure that error codes are set iff parsing fails.

Bug: 827551
Change-Id: I134199ca4538f469fc29d0b315a9e36f72e8df80
Reviewed-on: https://chromium-review.googlesource.com/982611Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548108}

components/cbor/cbor_reader.cc

@@ -51,6 +51,7 @@ const char kUnsupportedFloatingPointValue[] =
     "Floating point numbers are not supported.";
 const char kOutOfRangeIntegerValue[] =
     "Integer values must be between INT64_MIN and INT64_MAX.";
+const char kUnknownError[] = "An unknown error occured.";
 
 }  // namespace
 
@@ -88,15 +89,16 @@ base::Optional<CBORValue> CBORReader::Read(base::span<uint8_t const> data,
   base::Optional<CBORValue> decoded_cbor =
       reader.DecodeCompleteDataItem(max_nesting_level);
 
-  if (error_code_out)
-    *error_code_out = reader.GetErrorCode();
+  auto error_code = reader.GetErrorCode();
+  const bool failed = !decoded_cbor.has_value();
 
-  if (reader.GetErrorCode() != DecoderError::CBOR_NO_ERROR) {
-    *num_bytes_consumed = 0;
-    return base::nullopt;
-  }
+  // An error code must be set iff parsing failed.
+  DCHECK_EQ(failed, error_code != DecoderError::CBOR_NO_ERROR);
 
-  *num_bytes_consumed = reader.num_bytes_consumed();
+  if (error_code_out)
+    *error_code_out = error_code;
+
+  *num_bytes_consumed = failed ? 0 : reader.num_bytes_consumed();
   return decoded_cbor;
 }
 
@@ -395,6 +397,8 @@ const char* CBORReader::ErrorCodeToString(DecoderError error) {
       return kUnsupportedFloatingPointValue;
     case DecoderError::OUT_OF_RANGE_INTEGER_VALUE:
       return kOutOfRangeIntegerValue;
+    case DecoderError::UNKNOWN_ERROR:
+      return kUnknownError;
     default:
       NOTREACHED();
       return "Unknown error code.";

components/cbor/cbor_reader.h

@@ -66,6 +66,7 @@ class CBOR_EXPORT CBORReader {
     UNSUPPORTED_SIMPLE_VALUE,
     UNSUPPORTED_FLOATING_POINT_VALUE,
     OUT_OF_RANGE_INTEGER_VALUE,
+    UNKNOWN_ERROR,
   };
 
   // Encapsulates information extracted from the header of a CBOR data item,
@@ -96,7 +97,7 @@ class CBOR_EXPORT CBORReader {
   // formats is violated -including unknown additional info and incomplete
   // CBOR data- then an empty optional is returned. Optional |error_code_out|
   // can be provided by the caller to obtain additional information about
-  // decoding failures.
+  // decoding failures, which is always available if an empty value is returned.
   //
   // Fails if not all the data was consumed and sets |error_code_out| to
   // EXTRANEOUS_DATA in this case.
@@ -113,9 +114,10 @@ class CBOR_EXPORT CBORReader {
 
   // Reads and parses the header of CBOR data item from |input_data|. Optional
   // |error_code_out| can be provided by the caller to obtain additional
-  // information about decoding failures. Never fails with EXTRANEOUS_DATA, but
-  // informs the caller of how many bytes were consumed through
-  // |num_bytes_consumed|.
+  // information about decoding failures, which is always available if an empty
+  // value is returned. Never fails with EXTRANEOUS_DATA, but informs the
+  // caller of how many bytes were consumed through |num_bytes_consumed|, which
+  // is set to zero on error.
   static base::Optional<DataItemHeader> ReadDataItemHeader(
       base::span<const uint8_t> input_data,
       size_t* num_bytes_consumed = nullptr,