During a config reload, it is possible for the server to send
an -ERR with auth violation and then close the connection.
Client library most of the time will process the -ERR but in
some cases, the socket read gets an error before that can happen.
Some tests were expectign the async error handler to fire, and would
fail the test otherwise. Changed those tests to still check that
if the async error is fire, we get the expected error, but not fail
the test if we don't. We still must get the disconnected callback
in those cases though.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
- Config reload tests have been modified to not rely on symlink.
- Close logger on shutdown (for Windows tests cleanup)
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Needed to use filepath, not path for Join/Base in order for that
to work. Tests were actually failing on Windows and now they pass.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This is an issue in master only, not in any public release.
The issue is that permissions should be assigned as-is for the
route perms because Publish/Subscribe could be nil, so trying
to dereference Publish.Allow/Deny or Subscribe.Allow/Deny could
crash. The code checking for permissions correctly check if
Publish/Subscribe is nil or not.
This was introduced with PR #725
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
for the process name and service name. This allows the reuse of this
code in NATS Streaming Server by invoking the new setters to
change the service and process names.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
I don't think it is a good thing to compare the pointers and we
should use the DeepEqual instead.
When comparing a solicited route's URL to the URL that was created
during the parsing of the configuration, the pointers maybe the
same and so u1 == u2 would work. However, there are cases where
the URL is built on the fly based on the received route INFO protocol
so I think it is safer to use a function that does a reflect.DeepEqual
instead.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
When changing something in the cluster, such as Timeout and doing
a config reload, the route could be closed with an `Authorization
Error` report. Moreover, the route would not try to reconnect,
even if specified as an explicit route.
There were 2 issues:
- When checking if a solicited route is still valid, we need to
check the Routes' URL against the URL that we try to connect
to but not compare the pointers, but either do a reflect
deep equal, or compare their String representation (this is
what I do in the PR).
- We should check route authorization only if this is an accepted
route, not an explicit one. The reason is that we a server
explicitly connect to another server, it does not get the remote
server's username and password. So the check would always fail.
Note: It is possible that a config reload even without any change
in the cluster triggers the code checking if routes are properly
authorized, and that happens if there is TLS specified. When
the reload code checks if config has changed, the TLSConfig
between the old and new seem to indicate a change, eventhough there
is apparently none. Another reload does not detect a change. I
suspect some internal state in TLSConfig that causes the
reflect.DeepEqual() to report a difference.
Note2: This commit also contains fixes to regex that staticcheck
would otherwise complain about (they did not have any special
character), and I have removed printing the usage on startup when
getting an error. The usage is still correctly printed if passing
a parameter that is unknown.
Resolves#719
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
