This should fix#27726. With enough load, it might happen that the SetPongHandler
callback gets invoked before the call to SetReadDeadline is made in pingLoop. When
this occurs, the socket will end up with a 30s read deadline even though it got the pong,
which will lead to a timeout.
The fix here is processing the pong on pingLoop, synchronizing with the code that
sends the ping.
This PR adds server-side limits for JSON-RPC batch requests. Before this change, batches
were limited only by processing time. The server would pick calls from the batch and
answer them until the response timeout occurred, then stop processing the remaining batch
items.
Here, we are adding two additional limits which can be configured:
- the 'item limit': batches can have at most N items
- the 'response size limit': batches can contain at most X response bytes
These limits are optional in package rpc. In Geth, we set a default limit of 1000 items
and 25MB response size.
When a batch goes over the limit, an error response is returned to the client. However,
doing this correctly isn't always possible. In JSON-RPC, only method calls with a valid
`id` can be responded to. Since batches may also contain non-call messages or
notifications, the best effort thing we can do to report an error with the batch itself is
reporting the limit violation as an error for the first method call in the batch. If a batch is
too large, but contains only notifications and responses, the error will be reported with
a null `id`.
The RPC client was also changed so it can deal with errors resulting from too large
batches. An older client connected to the server code in this PR could get stuck
until the request timeout occurred when the batch is too large. **Upgrading to a version
of the RPC client containing this change is strongly recommended to avoid timeout issues.**
For some weird reason, when writing the original client implementation, @fjl worked off of
the assumption that responses could be distributed across batches arbitrarily. So for a
batch request containing requests `[A B C]`, the server could respond with `[A B C]` but
also with `[A B] [C]` or even `[A] [B] [C]` and it wouldn't make a difference to the
client.
So in the implementation of BatchCallContext, the client waited for all requests in the
batch individually. If the server didn't respond to some of the requests in the batch, the
client would eventually just time out (if a context was used).
With the addition of batch limits into the server, we anticipate that people will hit this
kind of error way more often. To handle this properly, the client now waits for a single
response batch and expects it to contain all responses to the requests.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
Here we add special handling for sending an error response when the write timeout of the
HTTP server is just about to expire. This is surprisingly difficult to get right, since is
must be ensured that all output is fully flushed in time, which needs support from
multiple levels of the RPC handler stack:
The timeout response can't use chunked transfer-encoding because there is no way to write
the final terminating chunk. net/http writes it when the topmost handler returns, but the
timeout will already be over by the time that happens. We decided to disable chunked
encoding by setting content-length explicitly.
Gzip compression must also be disabled for timeout responses because we don't know the
true content-length before compressing all output, i.e. compression would reintroduce
chunked transfer-encoding.
This adds a generic mechanism for 'dial options' in the RPC client,
and also implements a specific dial option for the JWT authentication
mechanism used by the engine API. Some real tests for the server-side
authentication handling are also added.
Co-authored-by: Joshua Gutow <jgutow@optimism.io>
Co-authored-by: Felix Lange <fjl@twurst.com>
This replaces the sketchy and undocumented string context keys for HTTP requests
with a defined interface. Using string keys with context is discouraged because
they may clash with keys created by other packages.
We added these keys to make connection metadata available in the signer, so this
change also updates signer/core to use the new PeerInfo API.
* Only compare hostnames in ws.origins
Also using a helper function for ToLower consolidates all preparation steps in one function for more maintainable consistency.
Spaces => tabs
Remove a semicolon
Add space at start of comment
Remove parens around conditional
Handle case wehre parsed hostname is empty
When passing a single word like "localhost" the parsed hostname is an empty string. Handle this and the error-parsing case together as default, and the nonempty hostname case in the conditional.
Refactor with new originIsAllowed functions
Adds originIsAllowed() & ruleAllowsOrigin(); removes prepOriginForComparison
Remove blank line
Added tests for simple allowed-orign rule
which does not specify a protocol or port, just a hostname
Fix copy-paste: `:=` => `=`
Remove parens around conditional
Remove autoadded whitespace on blank lines
Compare scheme, hostname, and port with rule
if the rule specifies those portions.
Remove one autoadded trailing whitespace
Better handle case where only origin host is given
e.g. "localhost"
Remove parens around conditional
Refactor: attemptWebsocketConnectionFromOrigin DRY
Include return type on helper function
Provide srv obj in helper fn
Provide srv to helper fn
Remove stray underscore
Remove blank line
parent 93e666b4c1e7e49b8406dc83ed93f4a02ea49ac1
author wbt <wbt@users.noreply.github.com> 1598559718 -0400
committer Martin Holst Swende <martin@swende.se> 1605602257 +0100
gpgsig -----BEGIN PGP SIGNATURE-----
iQFFBAABCAAvFiEEypmrtbNuJK1doP1AaDtDjAWl3fAFAl+zi9ARHG1hcnRpbkBz
d2VuZGUuc2UACgkQaDtDjAWl3fDRiwgAoMtzU8dwRV7Q9xkCwWEx9Wz2f3n6jUr2
VWBycDKGKwRkPPOER3oc9kzjGU/P1tFlK07PjfnAKZ9KWzxpDcJZwYM3xCBurG7A
16y4YsQnzgPNONv3xIkdi3RZtDBIiPFFEmdZFFvZ/jKexfI6JIYPngCAoqdTIFb9
On/aPvvVWQn1ExfmarsvvJ7kUDUG77tZipuacEH5FfFsfelBWOEYPe+I9ToUHskv
+qO6rOkV1Ojk8eBc6o0R1PnApwCAlEhJs7aM/SEOg4B4ZJJneiFuEXBIG9+0yS2I
NOicuDPLGucOB5nBsfIKI3USPeE+3jxdT8go2lN5Nrhm6MimoILDsQ==
=sgUp
-----END PGP SIGNATURE-----
Refactor: drop err var for more concise test lines
Add several tests for new WebSocket origin checks
Remove autoadded whitespace on blank lines
Restore TestWebsocketOrigins originally-named test
and rename the others to be helpers rather than full tests
Remove autoadded whitespace on blank line
Temporarily comment out new test sets
Uncomment test around origin rule with scheme
Remove tests without scheme on browser origin
per https://github.com/ethereum/go-ethereum/pull/21481/files#r479371498
Uncomment tests with port; remove some blank lines
Handle when browser does not specify scheme/port
Uncomment test for including scheme & port in rule
Add IP tests
* node: more tests + table-driven, ws origin changes
Co-authored-by: Martin Holst Swende <martin@swende.se>
This change makes it possible to run geth with JSON-RPC over HTTP and
WebSocket on the same TCP port. The default port for WebSocket
is still 8546.
geth --rpc --rpcport 8545 --ws --wsport 8545
This also removes a lot of deprecated API surface from package rpc.
The rpc package is now purely about serving JSON-RPC and no longer
provides a way to start an HTTP server.
This commit intents to replicate the DialHTTPWithClient function which allows
creating a RPC Client using a custom dialer but for websockets.
We introduce a new DialWebsocketWithDialer function which allows the caller
to instantiate a new websocket client using a custom dialer.
* rpc: improve codec abstraction
rpc.ServerCodec is an opaque interface. There was only one way to get a
codec using existing APIs: rpc.NewJSONCodec. This change exports
newCodec (as NewFuncCodec) and NewJSONCodec (as NewCodec). It also makes
all codec methods non-public to avoid showing internals in godoc.
While here, remove codec options in tests because they are not
supported anymore.
* p2p/simulations: use github.com/gorilla/websocket
This package was the last remaining user of golang.org/x/net/websocket.
Migrating to the new library wasn't straightforward because it is no
longer possible to treat WebSocket connections as a net.Conn.
* vendor: delete golang.org/x/net/websocket
* rpc: fix godoc comments and run gofmt
* rpc: implement websockets with github.com/gorilla/websocket
This change makes package rpc use the github.com/gorilla/websocket
package for WebSockets instead of golang.org/x/net/websocket. The new
library is more robust and supports all WebSocket features including
continuation frames.
There are new tests for two issues with the previously-used library:
- TestWebsocketClientPing checks handling of Ping frames.
- TestWebsocketLargeCall checks whether the request size limit is
applied correctly.
* rpc: raise HTTP/WebSocket request size limit to 5MB
* rpc: remove default origin for client connections
The client used to put the local hostname into the Origin header because
the server wanted an origin to accept the connection, but that's silly:
Origin is for browsers/websites. The nobody would whitelist a particular
hostname.
Now that the server doesn't need Origin anymore, don't bother setting
one for clients. Users who need an origin can use DialWebsocket to
create a client with arbitrary origin if needed.
* vendor: put golang.org/x/net/websocket back
* rpc: don't set Origin header for empty (default) origin
* rpc: add HTTP status code to handshake error
This makes it easier to debug failing connections.
* ethstats: use github.com/gorilla/websocket
* rpc: fix lint
New APIs added:
client.RegisterName(namespace, service) // makes service available to server
client.Notify(ctx, method, args...) // sends a notification
ClientFromContext(ctx) // to get a client in handler method
This is essentially a rewrite of the server-side code. JSON-RPC
processing code is now the same on both server and client side. Many
minor issues were fixed in the process and there is a new test suite for
JSON-RPC spec compliance (and non-compliance in some cases).
List of behavior changes:
- Method handlers are now called with a per-request context instead of a
per-connection context. The context is canceled right after the method
returns.
- Subscription error channels are always closed when the connection
ends. There is no need to also wait on the Notifier's Closed channel
to detect whether the subscription has ended.
- Client now omits "params" instead of sending "params": null when there
are no arguments to a call. The previous behavior was not compliant
with the spec. The server still accepts "params": null.
- Floating point numbers are allowed as "id". The spec doesn't allow
them, but we handle request "id" as json.RawMessage and guarantee that
the same number will be sent back.
- Logging is improved significantly. There is now a message at DEBUG
level for each RPC call served.
Currently http cors and websocket origins are a comma separated string in the
config object. These are replaced with string arrays that are more expressive in
case of a config file.
There is no need to depend on the old context package now that the
minimum Go version is 1.7. The move to "context" eliminates our weird
vendoring setup. Some vendored code still uses golang.org/x/net/context
and it is now vendored in the normal way.
This change triggered new vet checks around context.WithTimeout which
didn't fire with golang.org/x/net/context.
rpc: be less restrictive on the request id
rpc: improved documentation
console: upgrade web3.js to version 0.16.0
rpc: cache http connections
rpc: rename wsDomains parameter to wsOrigins
tylerni7
Felix Lange
mmsqe
oseau
Jonathan Otto
Sina Mahmoodi
Wihan de Beer
Jolly Zhao
protolambda
Jordan Krage
baptiste-b-pegasys
wbt
rene
Chris Pacia
Felix Lange
Peter Broadhurst
Ralph Caraveo III
Péter Szilágyi
Benoit Verkindt
bas-vk
Bas van Kervel