all: use net/http instead of x/net/http2

[leighmcculloch]

PR Checklist

PR Structure

• This PR has reasonably narrow scope (if not, break it down into smaller PRs).
• This PR avoids mixing refactoring changes with feature changes (split into two PRs
  otherwise).
• This PR's title starts with name of package that is most changed in the PR, ex.
  services/friendbot, or all or doc if the changes are broad or impact many
  packages.

Thoroughness

• This PR adds tests for the most critical parts of the new functionality or fixes.
• I've updated any docs (developer docs, .md
  files, etc... affected by this change). Take a look in the docs folder for a given service,
  like this one.

Release planning

• I've updated the relevant CHANGELOG (here for Horizon) if
  needed with deprecations, added features, breaking changes, and DB schema changes.
• I've decided if this PR requires a new major/minor version according to
  semver, or if it's mainly a patch change. The PR is targeted at the next
  release branch if it's not a patch change.

Summary

Use the default net/http transport which supports http2, instead of using x/net/http2.

Goal and scope

At some point early on we adopted using x/net/http2 as the http2 transport in our servers. It was probably necessary at the time, I'm not sure, but it's not necessary today unless we need full access to configure the transport. In our case we're initializing the http2 package with an empty config, so that alone suggests we are unnecessarily using that package. @bartekn noticed this while reviewing #1688 (comment) a dependency upgrade required for the shift from Dep to Modules.

The net/http package uses a vendored version of x/net/http2 for http2. We only need to use x/net/http2 directly if we want to customize it in ways that net/http doesn't expose, or if we want to use a later version of the http2 transport that has yet to be included in a release of Go. A huge downside of using the package directly though is that if critical fixes are made to it and included in a minor Go release we won't get them unless we manually update this package.

The godocs for the net/http package contain more details about the relationship between net/http and x/net/http2.

Close #1695

Summary of changes

• Remove configuration of the http2 transport for Horizon.
• Remove configuration of the http2 transport for code shared by Federation, Bifrost, and Bridge.

Testing

No tests that we have in the repository will identify any issues with this change, and any tests we could write would be complex and not helpful longterm, so I took a manual approach:

1. I ran Horizon with TLS enabled, with and without this change, and used curl to verify that the HTTP2 upgrade occurs successfully and results in a HTTP/2 200 response, using this curl command:

   curl -vk --http2 https://localhost:8000
   

   This test was successful.

2. I ran Horizon with TLS enabled, with and without this change, and used h2spec to verify that the server before and after the change has the same spec passes and failures, using this h2spec command:

   h2spec http2 -t -k -h=localhost -p=8000
   

   This test was successful on most runs. There were definitely spec failures, but they were consistent before and after the change. In one run I noticed spec 6.9.3 part 3 (window updates) failed on net/http, however on re-runs it didn't fail. Investigating, I found a report in x/net/http2: sendWindowUpdate may send invalid window size increment golang/go#31170 that x/net/http2 may not be handling window updates correctly in some scenarios, and so it is likely that this issue is intermittent and present when using either and I just witnessed it only on one.

An identical change was also made to code shared by the federation, bifrost, and bridge servers. I ran the same tests above against the federation server with TLS enabled to verify that both code paths were unaffected.

Known limitations & issues

N/A

What shouldn't be reviewed

N/A

[codecov-io]

Codecov Report

Merging #1713 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1713      +/-   ##
==========================================
- Coverage   45.15%   45.14%   -0.02%     
==========================================
  Files         381      381              
  Lines       25770    25758      -12     
==========================================
- Hits        11637    11629       -8     
+ Misses      12956    12953       -3     
+ Partials     1177     1176       -1

Impacted Files	Coverage Δ
support/http/main.go	37.5% <ø> (+1.13%)	⬆️
services/horizon/internal/app.go	57.45% <ø> (+0.25%)	⬆️
clients/horizonclient/internal.go	77.64% <0%> (-1.01%)	⬇️
clients/horizonclient/client.go	71.95% <0%> (+0.1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 947d54f...f24c5d2. Read the comment docs.