fix(compatibility): Replace or add RPC content type header when applicable

[oxarbitrage]

Motivation

We want Zebra to support the curl examples from the zcashd rpc documentation like https://zcash.github.io/rpc/getblockchaininfo.html where the content type header is text/plain.

Close #6363

Solution

The solution here actually ignores any content type sent by the client and always use application/json which is the only type we support. I could be wrong here.

Review

Anyone can review.

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

[oxarbitrage]

Before the change:

$ curl --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getblockchaininfo", "params": [] }' -H 'content-type: text/plain;' http://127.0.0.1:6666/
Supplied content type is not allowed. Content-Type: application/json is required
$ 

After:

$ curl --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getblockchaininfo", "params": [] }' -H 'content-type: text/plain;' http://127.0.0.1:6666/
{"result":{"chain":"main","blocks":11793,"bestblockhash":"00000001036cd5b8c7730a662a4a7533c4cacc0b0a8989f629ea4e6a754acbf2","estimatedheight":2128414,"upgrades":{"5ba81b19":{"name":"Overwinter","activationheight":347500,"status":"pending"},"76b809bb":{"name":"Sapling","activationheight":419200,"status":"pending"},"2bb40e60":{"name":"Blossom","activationheight":653600,"status":"pending"},"f5b9230b":{"name":"Heartwood","activationheight":903000,"status":"pending"},"e9ff75a6":{"name":"Canopy","activationheight":1046400,"status":"pending"},"c2d6d0b4":{"name":"Nu5","activationheight":1687104,"status":"pending"}},"consensus":{"chaintip":"00000000","nextblock":"00000000"}},"id":"curltest"}
$ curl --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getblockchaininfo", "params": [] }' http://127.0.0.1:6666/
{"result":{"chain":"main","blocks":11843,"bestblockhash":"00000000d3dccd08c7b4a1d56cb7d34462cd686ccd5f866490168b6ed36cefe6","estimatedheight":2128419,"upgrades":{"5ba81b19":{"name":"Overwinter","activationheight":347500,"status":"pending"},"76b809bb":{"name":"Sapling","activationheight":419200,"status":"pending"},"2bb40e60":{"name":"Blossom","activationheight":653600,"status":"pending"},"f5b9230b":{"name":"Heartwood","activationheight":903000,"status":"pending"},"e9ff75a6":{"name":"Canopy","activationheight":1046400,"status":"pending"},"c2d6d0b4":{"name":"Nu5","activationheight":1687104,"status":"pending"}},"consensus":{"chaintip":"00000000","nextblock":"00000000"}},"id":"curltest"}
$

Zcash-cli after the change:

$ ./zcash-cli -rpcport=6666 getblockchaininfo
{
  "chain": "main",
  "blocks": 20952,
  "bestblockhash": "00000000511a302da9b0de8ec02e744b39a7ffe3e0deda88bae493d574ecf9df",
  "estimatedheight": 2128470,
  "upgrades": {
    "5ba81b19": {
      "name": "Overwinter",
      "activationheight": 347500,
      "status": "pending"
    },
    "76b809bb": {
      "name": "Sapling",
      "activationheight": 419200,
      "status": "pending"
    },
    "2bb40e60": {
      "name": "Blossom",
      "activationheight": 653600,
      "status": "pending"
    },
    "f5b9230b": {
      "name": "Heartwood",
      "activationheight": 903000,
      "status": "pending"
    },
    "e9ff75a6": {
      "name": "Canopy",
      "activationheight": 1046400,
      "status": "pending"
    },
    "c2d6d0b4": {
      "name": "Nu5",
      "activationheight": 1687104,
      "status": "pending"
    }
  },
  "consensus": {
    "chaintip": "00000000",
    "nextblock": "00000000"
  }
}
$

[teor2345]

I've had a look at this PR, and it solves the compatibility problem in a really neat way.

But it also potentially introduces other security issues.

content-type headers exist so that applications know they are speaking the correct protocol with the correct format. We can be a bit flexible, but there are some types (such as binary) we shouldn't allow.

In particular, the "application/x-www-form-urlencoded" header should be rejected, so browser forms can't be used to attack a local RPC port. See "The Role of Routers in the CSRF Attack" in:
https://www.invicti.com/blog/web-security/importance-content-type-header-http-requests/

[codecov]

Codecov Report

Merging #6885 (ddf5067) into main (9959a6c) will decrease coverage by 0.27%.
The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6885      +/-   ##
==========================================
- Coverage   77.72%   77.46%   -0.27%     
==========================================
  Files         310      310              
  Lines       41416    41523     +107     
==========================================
- Hits        32192    32167      -25     
- Misses       9224     9356     +132     

[teor2345]

Looks good!

The security comments are needed so we know why we only replaced the text/plain header. The code cleanup is optional.

I've labelled this PR so it doesn't merge yet, because we started a stable release freeze on Friday, and this isn't a release blocker.

[teor2345]

Zcash-cli after the change:

Can you do these manual tests again when you've finished changing the code?

[teor2345]

Thanks, this is good to go as soon as we've re-done the manual tests. (And tagged the stable release.)

[oxarbitrage]

it seems we have a problem, the zcashd documentation use text/plain; in their curl examples instead of text/plain which is the string are looking for. The easier thing to do will be to replace it but i was wondering if we should support both ?

[teor2345]

it seems we have a problem, the zcashd documentation use text/plain; in their curl examples instead of text/plain which is the string are looking for. The easier thing to do will be to replace it but i was wondering if we should support both ?

Good question!

We could replace anything that starts with text/plain, and add these common test cases to the tests:

• text/plain
• text/plain;
• text/plain; charset=utf-8

https://stackoverflow.com/questions/30262241/specifying-a-character-set-in-http-headers/30266220#30266220

[teor2345]

You could use:
https://doc.rust-lang.org/std/primitive.str.html#method.starts_with

[teor2345]

Thanks for the fix and the extra tests!