Improve endpoint handling. #11236
Improve endpoint handling. #11236
Conversation
- add tests
|
There is an inconsistency between arangosh --javascript.endpoints-whitelist "ssl://www\.arangodb\.com:443\$"
require("internal").download("https://www.arangodb.com") // allowed
require("internal").download("https://www.arangodb.com:443") // allowed
arangosh --javascript.endpoints-whitelist "https://www\.arangodb\.com:443\$"
require("internal").download("https://www.arangodb.com") // NOT allowed!
require("internal").download("https://www.arangodb.com:443") // allowedNot directly related, but odd: there is a pseudo-protocol(?) 127.0.0.1:8529@_system> require("internal").download("http://asdf.foo/path")
{
"code" : 500,
"message" : "Could not connect to 'http+tcp://asdf.foo:80' 'getaddrinfo for host 'asdf.foo': Name does not resolve'"
}
127.0.0.1:8529@_system> require("internal").download("srv://asdf.foo/path")
{
"code" : 500,
"message" : "Could not connect to 'asdf.foo' ''"
} |
You could make the port optional in the 2nd regular expression! The first rule apparently works with the endpoint check only and is important to be backward compatible. We should really encourage users just to use the
The failure shown here is not an permission error. Therefor it will not be fixed in this PR. |
|
Copied from original #11232 which was changed so something else since then: Requests may get redirected to a different endpoint. Let the user know the endpoint in case it is disallowed by endpoint black/whitelist for easier diagnosis, e.g. Related: arangodb/docs#92 Don 8000 e in 912fe72#diff-cf342f102b1a80552625f5193a113b4bR999 and eb6d153 |
…telisting * origin/devel: (80 commits) Feature/aql subquery execution block impl execute implementation batch sub queries (#11318) Fix isAdminUser. (#11326) Bug fix/fixes 20200318 (#11319) updated CHANGELOG Feature/out of search in range (#11324) fix "fix" for collection figures (#11323) updated CHANGELOG compilation fixes for clang-10s more strict checking (#11316) fix failing query (#11317) KShortestPathsExecutor must reset its KShortestPathFinder, including all caches. (#11312) Feature/aql subquery execution block impl execute implementation expected number of rows (#11274) Add DTRACE points to measure request timings. (#11245) USE_STRICT_OPENSSL is Off by default Fix usesRevisionAsDocumentId population and add syncByRevision flag (#11314) Traversal Bugfix (#11310) Bug fix/issue 11275 (#11299) added simple test (#11301) Fix some typos (#10173) Documentation/typos 2020-01-24 (#10975) Update CHANGELOG ...
lib/V8/v8-utils.cpp
Outdated
| if (endpoint.find(':') == std::string::npos) { | ||
| endpoint.append(":80"); | ||
| } | ||
| endpoint = "tcp://" + endpoint; |
There was a problem hiding this comment.
shouldn't this be
| endpoint = "tcp://" + endpoint; | |
| endpoint = "ssl://" + endpoint; |
There was a problem hiding this comment.
I am not sure the code is just moved up and it was in the original code like that. Maybe h2c does the initial connection over http. Maybe @graetzer has an opinion on this.
There was a problem hiding this comment.
simon:arango: 15:48
kp kann man wahrscheinlich ssl:// draus machen wenn das h2s vorher war
15:48
h2s ist eh nur unser propiertäres ding um http2 einzuschalten
I am not sure what to make out of this. Do you want me switch it to ssl?
There was a problem hiding this comment.
Should the port be changed as well?
lib/V8/v8-utils.cpp
Outdated
| relative = "/" + relative; | ||
| } | ||
| if (endpoint.find(':') == std::string::npos) { | ||
| endpoint.append(":80"); |
There was a problem hiding this comment.
shouldn't we also move to port 443 here?
…ql-functions * origin/devel: Bug fix/schema validation return code (#11341) Fix explainer output when restricting collections (#11338) remove tabstops Improve endpoint handling. (#11236) revert HTTP return code change for user API, add tests (#11331) remove unused header files (#11320) Feature/aql subquery execution block impl execute implementation batch sub queries (#11318) Fix isAdminUser. (#11326)
tcp:// and ssl:// are still supported but somehow buggy 67DE . Updated docs only describe http:// and https:// that can now be used thanks to arangodb/arangodb#11236 h2://, h2s://, srv:// and unix:// are supported as well but not left uncovered in the docs.
…st options (#352) Adjusted to actual behavior of endpoint filtering, some other adjustments. tcp:// and ssl:// are still supported but somehow buggy. Updated docs only describe http:// and https:// that can now be used thanks to arangodb/arangodb#11236 h2://, h2s://, srv:// and unix:// are supported as well but not left uncovered in the docs.