node: serve JSON-RPC on custom path prefix

[renaynay]

This PR allows a user to set a custom path prefix on which to mount the http-rpc or ws-rpc handlers via the new flags --http.prefix and --ws.prefix. A user will also be able to set a custom path prefix if starting up RPC or WS through the console.

If a custom path is not set, the default is to mount the handler on the root path. If / is specified as either the http or ws prefix, all paths will be served.

This PR is an implementation of the issue addressed in #21826.

[renaynay]

@ryanschneider here's the final PR, please let me know if this solves your issues!

[ryanschneider]

@renaynay I tried it out but was not able to get geth to behave how geth behaved in older versions where it would accept on all paths. I tried --http.path /, --http.path /* and leaving the argument off, but in all 3 cases hitting /foo led to a 404.

Also, with the default path there's still the bug that hitting /?foo=bar returns a 404 when ii (IMO) shouldn't, though this does not seem to be the case when you set a non-default path.

[renaynay]

@ryanschneider Ah yes I see. Okay I will set a special case for if the path is defined as * or /* to serve on all paths below root then. If the path is default though (just root), then it will not serve RPC beyond the root unless /* or * are specified for http.path.

[fjl]

tried it out but was not able to get geth to behave how geth behaved in older versions where it would accept on all paths.

@ryanschneider that's not really the intention here. The idea was to provide a way for you guys to set the path
that your AWS loadbalancer uses. In #21826 (comment),
you wrote that requests go to http://internal-loadbalancer/mainnet/archive for some reason. From this,
I assumed that you'd be able to know the path that will be used for each of your geth instances, so for your
mainnet archive node, you'd use --http.path /mainnet/archive.

We can't make it serve JSON-RPC on all paths by default, since this will conflict with other resources on
the same server. So the default behavior of geth should be that JSON-RPC is only available on /, and
not available on other paths.

The query string issue is something we should fix, regardless of the path. It should just ignore the query
when checking the path.

The way @renaynay implemented it here is a bit special-casey, since it makes -http.path check for
an exact match with /, and a prefix match with all other paths. I would strongly prefer to not make
it a prefix match, but if it absolutely must be a prefix to please your setup, we should probably go ahead
and ensure that --http.path / makes it serve JSON-RPC on all paths.

[ryanschneider]

@fjl as I mentioned in #21826 (comment) later we'd still really want to have the option to support prefixes, ideally the / prefix, since as I stated "want to move geth nodes around without having to reconfigure them so as long as that was a supported prefix this idea [allowing a configurable path] would work for us"

@renaynay's original PR worked for us, it's the changes made in response to #21826 (comment) during the back and forth that caused it to "regress" for our use case.

My main point is that we saw value in the original pre .12 behavior of accepting JSONRPC on any path, and would like a way to configure that behavior again (it doesn't need to be the default behavior though).

[renaynay]

Okay so, how about I add the * wildcard option so when a user specified --http.path=/*, then rpc will be served over any path. Would this work for you @ryanschneider @fjl ? I will also fix the issue with the query strings.

[ryanschneider]

Maybe instead of parsing out the “*” it’d be easier to use gorilla and add an additional —http.prefix Boolean argument that controls whether the route is registered using .PathPrefix? It’s been awhile since I’ve tried setting up any “dynamic” routes like this so might not be the best approach though.

…

On Thu, Jan 21, 2021 at 6:25 AM rene ***@***.***> wrote: Okay so, how about I add the * wildcard option so when a user specified --http.path=/*, then rpc will be served over any path. Would this work for you @ryanschneider <https://github.com/ryanschneider> @fjl <https://github.com/fjl> ? I will also fix the issue with the query strings. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22184 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANCEEHH76WMFRYWAMRJFDS3A2O5ANCNFSM4WHAO7WQ> .

[renaynay]

@ryanschneider yeah I was considering using gorilla, but we are hesitant to introduce new dependencies into geth. I think we will do this manually. I could also add an --http.prefix bool flag to indicate whether the path should be treated as path or prefix.

[ryanschneider]

Sounds good to me.

…

On Thu, Jan 21, 2021 at 7:26 AM rene ***@***.***> wrote: @ryanschneider <https://github.com/ryanschneider> yeah I was considering using gorilla, but we are hesitant to introduce new dependencies into geth. I think we will do this manually. I could also add an --http.prefix bool flag to indicate whether the path should be treated as path or prefix. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22184 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANCEDMTVMSEQJ5GVDTTBDS3BBTLANCNFSM4WHAO7WQ> .

[renaynay]

@ryanschneider we have decided to consolidate the two flags into one --http.prefix flag (or --ws.prefix flag). This flag will set a custom prefix only. If a user specifies / as the prefix, all paths will be served. If this flag is not set, the default is to serve requests only on root.

Also the issue with query strings should now be solved.

Apologies for all the back and forth, I hope this solution is satisfactory.

[ryanschneider]

Ya this sounds good to me. Thanks for working on this!

…

On Mon, Jan 25, 2021 at 8:45 AM rene ***@***.***> wrote: @ryanschneider <https://github.com/ryanschneider> we have decided to consolidate the two flags into one --http.prefix flag (or --ws.prefix flag). This flag will set a custom prefix *only*. If a user specifies / as the prefix, all paths will be served. If this flag is not set, the default is to serve requests only on root. Apologies for all the back and forth, I hope this solution is satisfactory. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22184 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANCEBZNYON32ZKM4WMHGLS3WNZXANCNFSM4WHAO7WQ> .

[fjl]

I've made a couple changes, it should be good to go now:

• flag is renamed to --http.rpcprefix
• '/' in prefix is required, otherwise node constructor fails
• admin API / console does not expose the path setting. I don't want to change the API for this.

[fjl]

Updated again to add this error:

~/d/e/s/g/e/go-ethereum >> ./geth --syncmode=light --http --http.rpcprefix /foo?
INFO [02-01|15:00:26.485] Starting Geth on Ethereum mainnet... 
INFO [02-01|15:00:26.485] Dropping default light client cache      provided=1024 updated=128
INFO [02-01|15:00:26.486] Maximum peer count                       ETH=0 LES=10 total=50
Fatal: Failed to create the protocol stack: HTTP RPC path prefix "/foo?" contains URL meta-characters

[fjl]

Not sure if it's a good idea though. The '?' would match correctly, just not in the obvious way. If someone used --rpcpath /foo?, it would match requests with path /foo%3F, but not with path /foo?. Question is, should we permit this behavior or just make it impossible to configure?

[renaynay]

@fjl I think we should prohibit this behavior and not leave a lot of nuance / ambiguity. I like throwing the fatal err like you have implemented:

Fatal: Failed to create the protocol stack: HTTP RPC path prefix "/foo?" contains URL meta-characters

[fjl]

OK then, let's merge when CI turns green