Fix HTTP/REST clients HTTP Content-Type header parsers. (#4139)

* Fix client HTTP content-type parsers. * Fix tests. * Address review comment and apply wildcard checks for generic decodeBytes.
2022-09-19 12:17:29 +03:00 · 2022-09-19 12:17:29 +03:00 · ca871a5435
parent 9999362b11
commit ca871a5435
6 changed files with 131 additions and 109 deletions
--- a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim
+++ b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim
@ -70,6 +70,7 @@ const

  ApplicationJsonMediaType* = MediaType.init("application/json")
  TextPlainMediaType* = MediaType.init("text/plain")
+  OctetStreamMediaType* = MediaType.init("application/octet-stream")
  UrlEncodedMediaType* = MediaType.init("application/x-www-form-urlencoded")

 type
@ -2376,10 +2377,21 @@ proc encodeBytes*[T: EncodeArrays](value: T,
  else:
    err("Content-Type not supported")

-proc decodeBytes*[T: DecodeTypes](t: typedesc[T], value: openArray[byte],
-                                  contentType: string): RestResult[T] =
-  case contentType
-  of "application/json":
+proc decodeBytes*[T: DecodeTypes](
+       t: typedesc[T],
+       value: openArray[byte],
+       contentType: Opt[ContentTypeData]
+     ): RestResult[T] =
+
+  let mediaType =
+    if contentType.isNone():
+      ApplicationJsonMediaType
+    else:
+      if isWildCard(contentType.get().mediaType):
+        return err("Incorrect Content-Type")
+      contentType.get().mediaType
+
+  if mediaType == ApplicationJsonMediaType:
    try:
      ok RestJson.decode(value, T,
                         requireAllFields = true,
@ -2392,10 +2404,19 @@ proc decodeBytes*[T: DecodeTypes](t: typedesc[T], value: openArray[byte],
  else:
    err("Content-Type not supported")

-proc decodeBytes*[T: SszDecodeTypes](t: typedesc[T], value: openArray[byte],
-                                     contentType: string, updateRoot = true): RestResult[T] =
-  case contentType
-  of "application/octet-stream":
+proc decodeBytes*[T: SszDecodeTypes](
+       t: typedesc[T],
+       value: openArray[byte],
+       contentType: Opt[ContentTypeData],
+       updateRoot = true
+     ): RestResult[T] =
+
+  if contentType.isNone() or
+     isWildCard(contentType.get().mediaType):
+    return err("Missing or incorrect Content-Type")
+
+  let mediaType = contentType.get().mediaType
+  if mediaType == OctetStreamMediaType:
    try:
      var v: RestResult[T]
      v.ok(T()) # This optimistically avoids an expensive genericAssign
--- a/beacon_chain/spec/eth2_apis/rest_beacon_calls.nim
+++ b/beacon_chain/spec/eth2_apis/rest_beacon_calls.nim
@ -132,27 +132,23 @@ proc getBlock*(client: RestClientRef, block_id: BlockIdent,
  let data =
    case resp.status
    of 200:
-      case resp.contentType
-      of "application/json":
-        let blck =
-          block:
-            let res = decodeBytes(GetBlockResponse, resp.data,
-                                  resp.contentType)
-            if res.isErr():
-              raise newException(RestError, $res.error())
-            res.get()
-        ForkedSignedBeaconBlock.init(blck.data)
-      of "application/octet-stream":
-        let blck =
-          block:
-            let res = decodeBytes(GetPhase0BlockSszResponse, resp.data,
-                                  resp.contentType)
-            if res.isErr():
-              raise newException(RestError, $res.error())
-            res.get()
-        ForkedSignedBeaconBlock.init(blck)
+      if resp.contentType.isNone() or
+         isWildCard(resp.contentType.get().mediaType):
+        raise newException(RestError, "Missing or incorrect Content-Type")
      else:
-        raise newException(RestError, "Unsupported content-type")
+        let mediaType = resp.contentType.get().mediaType
+        if mediaType == ApplicationJsonMediaType:
+          let blck = decodeBytes(GetBlockResponse, resp.data,
+                                 resp.contentType).valueOr:
+            raise newException(RestError, $error)
+          ForkedSignedBeaconBlock.init(blck.data)
+        elif mediaType == OctetStreamMediaType:
+          let blck = decodeBytes(GetPhase0BlockSszResponse, resp.data,
+                                 resp.contentType).valueOr:
+            raise newException(RestError, $error)
+          ForkedSignedBeaconBlock.init(blck)
+        else:
+          raise newException(RestError, "Unsupported Content-Type")
    of 400, 404, 500:
      raiseGenericError(resp)
    else:
@ -180,35 +176,31 @@ proc getBlockV2*(client: RestClientRef, block_id: BlockIdent,
  return
    case resp.status
    of 200:
-      case resp.contentType
-      of "application/json":
-        let blck =
-          block:
-            let res = decodeBytes(GetBlockV2Response, resp.data,
-                                  resp.contentType)
-            if res.isErr():
-              raise newException(RestError, $res.error())
-            newClone(res.get())
-        some blck
-      of "application/octet-stream":
-        try:
-          some newClone(readSszForkedSignedBeaconBlock(cfg, resp.data))
-        except CatchableError as exc:
-          raise newException(RestError, exc.msg)
+      if resp.contentType.isNone() or
+         isWildCard(resp.contentType.get().mediaType):
+        raise newException(RestError, "Missing or incorrect Content-Type")
      else:
-        raise newException(RestError, "Unsupported content-type")
+        let mediaType = resp.contentType.get().mediaType
+        if mediaType == ApplicationJsonMediaType:
+          let blck = decodeBytes(GetBlockV2Response, resp.data,
+                                 resp.contentType).valueOr:
+            raise newException(RestError, $error)
+          some(newClone(blck))
+        elif mediaType == OctetStreamMediaType:
+          try:
+            some newClone(readSszForkedSignedBeaconBlock(cfg, resp.data))
+          except CatchableError as exc:
+            raise newException(RestError, exc.msg)
+        else:
+          raise newException(RestError, "Unsupported Content-Type")
    of 404:
      none(ref ForkedSignedBeaconBlock)
-
    of 400, 500:
-      let error =
-        block:
-          let res = decodeBytes(RestGenericError, resp.data, resp.contentType)
-          if res.isErr():
-            let msg = "Incorrect response error format (" & $resp.status &
-                      ") [" & $res.error() & "]"
-            raise newException(RestError, msg)
-          res.get()
+      let error = decodeBytes(RestGenericError, resp.data,
+                              resp.contentType).valueOr:
+        let msg = "Incorrect response error format (" & $resp.status &
+                  ") [" & $error & "]"
+        raise newException(RestError, msg)
      let msg = "Error response (" & $resp.status & ") [" & error.message & "]"
      raise newException(RestError, msg)
    else:
--- a/beacon_chain/spec/eth2_apis/rest_debug_calls.nim
+++ b/beacon_chain/spec/eth2_apis/rest_debug_calls.nim
@ -32,27 +32,23 @@ proc getState*(client: RestClientRef, state_id: StateIdent,
  let data =
    case resp.status
    of 200:
-      case resp.contentType
-      of "application/json":
-        let state =
-          block:
-            let res = decodeBytes(GetStateResponse, resp.data,
-                                  resp.contentType)
-            if res.isErr():
-              raise newException(RestError, $res.error())
-            res.get()
-        state.data
-      of "application/octet-stream":
-        let state =
-          block:
-            let res = decodeBytes(GetPhase0StateSszResponse, resp.data,
-                                  resp.contentType)
-            if res.isErr():
-              raise newException(RestError, $res.error())
-            res.get()
-        state
+      if resp.contentType.isNone() or
+         isWildCard(resp.contentType.get().mediaType):
+        raise newException(RestError, "Missing or incorrect Content-Type")
      else:
-        raise newException(RestError, "Unsupported content-type")
+        let mediaType = resp.contentType.get().mediaType
+        if mediaType == ApplicationJsonMediaType:
+          let state = decodeBytes(GetStateResponse, resp.data,
+                                  resp.contentType).valueOr:
+            raise newException(RestError, $error)
+          state.data
+        elif mediaType == OctetStreamMediaType:
+          let state = decodeBytes(GetPhase0StateSszResponse, resp.data,
+                                  resp.contentType).valueOr:
+            raise newException(RestError, $error)
+          state
+        else:
+          raise newException(RestError, "Unsupported content-type")
    of 400, 404, 500:
      let error =
        block:
@ -93,23 +89,27 @@ proc getStateV2*(client: RestClientRef, state_id: StateIdent,
  let data =
    case resp.status
    of 200:
-      case resp.contentType
-      of "application/json":
-        let state =
-          block:
-            let res = newClone(decodeBytes(GetStateV2Response, resp.data,
-                                  resp.contentType))
-            if res[].isErr():
-              raise newException(RestError, $res[].error())
-            newClone(res[].get())
-        state
-      of "application/octet-stream":
-        try:
-          newClone(readSszForkedHashedBeaconState(cfg, resp.data))
-        except CatchableError as exc:
-          raise newException(RestError, exc.msg)
+      if resp.contentType.isNone() or
+         isWildCard(resp.contentType.get().mediaType):
+        raise newException(RestError, "Missing or incorrect Content-Type")
      else:
-        raise newException(RestError, "Unsupported content-type")
+        let mediaType = resp.contentType.get().mediaType
+        if mediaType == ApplicationJsonMediaType:
+          let state =
+            block:
+              let res = newClone(decodeBytes(GetStateV2Response, resp.data,
+                                    resp.contentType))
+              if res[].isErr():
+                raise newException(RestError, $res[].error())
+              newClone(res[].get())
+          state
+        elif mediaType == OctetStreamMediaType:
+          try:
+            newClone(readSszForkedHashedBeaconState(cfg, resp.data))
+          except CatchableError as exc:
+            raise newException(RestError, exc.msg)
+        else:
+          raise newException(RestError, "Unsupported content-type")
    of 404:
      nil
    of 400, 500:
--- a/beacon_chain/spec/eth2_apis/rest_remote_signer_calls.nim
+++ b/beacon_chain/spec/eth2_apis/rest_remote_signer_calls.nim
@ -105,20 +105,29 @@ proc signData*(client: RestClientRef, identifier: ValidatorPubKey,
    case response.status
    of 200:
      inc(nbc_remote_signer_200_responses)
-      let sig = if response.contentType.contains("text/plain"):
-        let asStr = fromBytes(string, response.data)
-        let sigFromText = fromHex(ValidatorSig, asStr)
-        if sigFromText.isErr:
-          return Web3SignerDataResponse.err("Unable to decode signature from plain text")
-        sigFromText.get.load
-      else:
-        let res = decodeBytes(Web3SignerSignatureResponse, response.data,
-                              response.contentType)
-        if res.isErr:
-          let msg = "Unable to decode remote signer response [" & $res.error() & "]"
-          inc(nbc_remote_signer_failures)
-          return Web3SignerDataResponse.err(msg)
-        res.get.signature.load
+      let sig =
+        if response.contentType.isNone() or
+           isWildCard(response.contentType.get().mediaType):
+          return Web3SignerDataResponse.err(
+            "Unable to decode signature from missing or incorrect content")
+        else:
+          let mediaType = response.contentType.get().mediaType
+          if mediaType == TextPlainMediaType:
+            let asStr = fromBytes(string, response.data)
+            let sigFromText = fromHex(ValidatorSig, asStr)
+            if sigFromText.isErr:
+              return Web3SignerDataResponse.err(
+                "Unable to decode signature from plain text")
+            sigFromText.get.load
+          else:
+            let res = decodeBytes(Web3SignerSignatureResponse, response.data,
+                                  response.contentType)
+            if res.isErr:
+              let msg = "Unable to decode remote signer response [" &
+                        $res.error() & "]"
+              inc(nbc_remote_signer_failures)
+              return Web3SignerDataResponse.err(msg)
+            res.get.signature.load

      if sig.isNone:
        let msg = "Remote signer returns invalid signature"
--- a/tests/test_keymanager_api.nim
+++ b/tests/test_keymanager_api.nim
@ -650,19 +650,19 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
      let
        r1 = decodeBytes(KeystoresAndSlashingProtection,
                         Vector1.toOpenArrayByte(0, len(Vector1) - 1),
-                         "application/json")
+                         Opt.some(getContentType("application/json").get()))
        r2 = decodeBytes(KeystoresAndSlashingProtection,
                         Vector2.toOpenArrayByte(0, len(Vector2) - 1),
-                         "application/json")
+                         Opt.some(getContentType("application/json").get()))
        r3 = decodeBytes(KeystoresAndSlashingProtection,
                         Vector3.toOpenArrayByte(0, len(Vector3) - 1),
-                         "application/json")
+                         Opt.some(getContentType("application/json").get()))
        r4 = decodeBytes(KeystoresAndSlashingProtection,
                         Vector4.toOpenArrayByte(0, len(Vector4) - 1),
-                         "application/json")
+                         Opt.some(getContentType("application/json").get()))
        r5 = decodeBytes(KeystoresAndSlashingProtection,
                         Vector5.toOpenArrayByte(0, len(Vector5) - 1),
-                         "application/json")
+                         Opt.some(getContentType("application/json").get()))

      check:
        r1.isOk() == true
--- a/vendor/nim-presto
+++ b/vendor/nim-presto
@ -1 +1 @@
-Subproject commit 3984431dc0fc829eb668e12e57e90542b041d298
+Subproject commit 8d5a2512f0fbbb44d265f179c5c5769f4bc351ea