feat(net): follow http redirects

[tmpolaczyk]

Fix #2305

[tmpolaczyk]

If the redirect limit is reached, the data source will resolve to the RadError HttpOther { message: "number of redirects hit the maximum amount" }

[guidiaz]

If the redirect limit is reached, the data source will resolve to the RadError HttpOther { message: "number of redirects hit the maximum amount" }

Another option would be to resolve with the http error produced by last attempt. This way it the ultimate http error may be specifically refered as a specific enumerated value when reported to Solidity.

[tmpolaczyk]

Another option would be to resolve with the http error produced by last attempt. This way it the ultimate http error may be specifically refered as a specific enumerated value when reported to Solidity.

Yeah, I actually expected that behavior, but not sure if we can extract the http status code from that error.

[aesedepece]

If the redirect limit is reached, the data source will resolve to the RadError HttpOther { message: "number of redirects hit the maximum amount" }

Another option would be to resolve with the http error produced by last attempt. This way it the ultimate http error may be specifically refered as a specific enumerated value when reported to Solidity.

Redirects are not errors, so there's no such a thing as an error produced by last attempt. But hitting a client-chosen number of redirects is indeed a client error, which is the one to commit.

[tmpolaczyk]

Marking this as a draft because I'm not sure if we can release this without TAPI, as it may change the behavior of some requests.

[aesedepece]

Marking this as a draft because I'm not sure if we can release this without TAPI, as it may change the behavior of some requests.

It could indeed be potentially used as a griefing attack by crafting data requests that will use between 2 and 4 redirects and thus cause non-adopting nodes to commit an error and adopting ones to commit a value.

As a counterargument, as you can't easily use that attack in a targeted manner so as to intentionally steal collateral from someone else's identities into your own, the existence of that vector attack could arguably be considered pretty equivalent to all other existing "inconsistent source" attacks.

However this one wouldn't be prevented by the paranoid mode / retrieval proxies, so it's probably a good idea to use TAPI for this, just to stay on the safe side.

[aesedepece]

This change sounds like a good candidate for being shipped with 1.6 — behind a TAPI flag if necessary. What do you think?

[tmpolaczyk]

Seems like a good candidate for 1.6, with TAPI.

I believe the only unresolved issue is the error message when hitting the redirect limit? It is currently HttpOther { message: "number of redirects hit the maximum amount" }, I would expect it to return some HTTP status code in the 3xx range as well, but the http client doesn't give us that information.

[aesedepece]

I believe the only unresolved issue is the error message when hitting the redirect limit? It is currently HttpOther { message: "number of redirects hit the maximum amount" }, I would expect it to return some HTTP status code in the 3xx range as well, but the http client doesn't give us that information.

Funnily enough, there's no widely accepted HTTP client error code for "too many redirects" 🤷

Anyway, are you suggesting to have something like a dedicated RadError::HttpTooManyRedirects?

[aesedepece]

One way or another, I think we need a WIP here. So is that something @tmpolaczyk you're interested in working on? Or shall I go and create the WIP based on this implementation?

[tmpolaczyk]

I was suggesting something like HttpError { code: 302 }, just so that the end user can get the specific error code returned by the server. But it is hard for me to imagine any use cases that would need to detect HTTP redirects, so maybe the generic HttpOther error is fine.

And feel free to write the WIP!

[aesedepece]

We have a WIP now at https://github.com/witnet/WIPs/pull/93/files!