10000 Improve VST Auth errors handling (#9972) · distrubuted/arangodb@98c3b5c · GitHub
[go: up one dir, main page]
Skip to content

Commit 98c3b5c

Browse files
graetzerjsteemann
graetzer
authored and
jsteemann
committed
Improve VST Auth errors handling (arangodb#9972)
1 parent 77034c0 commit 98c3b5c

File tree

7 files changed

+56
-45
lines changed

7 files changed

+56
-45
lines changed

‎3rdParty/fuerte/include/fuerte/types.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ enum class Error : uint16_t {
7272
WriteError = 1103,
7373

7474
Canceled = 1104,
75+
76+
VstUnauthorized = 2000,
7577

7678
ProtocolError = 3000,
7779
};

‎3rdParty/fuerte/src/GeneralConnection.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ void GeneralConnection<ST>::startConnection() {
6767

6868
// shutdown the connection and cancel all pending messages.
6969
template <SocketType ST>
70-
void GeneralConnection<ST>::shutdownConnection(const Error err) {
71-
FUERTE_LOG_DEBUG << "shutdownConnection: this=" << this << "\n";
70+
void GeneralConnection<ST>::shutdownConnection(const Error err,
71+
std::string const& msg) {
72+
FUERTE_LOG_DEBUG << "shutdownConnection: '" << msg
73+
<< "' this=" << this << "\n";
7274

7375
if (_state.load() != Connection::State::Failed) {
7476
_state.store(Connection::State::Disconnected);
@@ -86,6 +88,8 @@ void GeneralConnection<ST>::shutdownConnection(const Error err) {
8688
}
8789

8890
abortOngoingRequests(err);
91+
92+
onFailure(err, msg);
8993

9094
// clear buffer of received messages
9195
_receiveBuffer.consume(_receiveBuffer.size());
@@ -125,9 +129,9 @@ void GeneralConnection<ST>::tryConnect(unsigned retries) {
125129
tryConnect(retries - 1);
126130
} else {
127131
_state.store(Connection::State::Failed, std::memory_order_release);
128-
shutdownConnection(Error::CouldNotConnect);
132+
shutdownConnection(Error::CouldNotConnect,
133+
"connecting failed: " + ec.message());
129134
drainQueue(Error::CouldNotConnect);
130-
onFailure(Error::CouldNotConnect, "connecting failed: " + ec.message());
131135
}
132136
});
133137
}

‎3rdParty/fuerte/src/GeneralConnection.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class GeneralConnection : public fuerte::Connection {
5353

5454
protected:
5555
// shutdown connection, cancel async operations
56-
void shutdownConnection(const fuerte::Error);
56+
void shutdownConnection(const fuerte::Error, std::string const& msg = "");
5757

5858
// Connect with a given number of retries
5959
void tryConnect(unsigned retries);

‎3rdParty/fuerte/src/HttpConnection.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ MessageID HttpConnection<ST>::sendRequest(std::unique_ptr<Request> req,
226226
this->startConnection();
227227
} else if (state == Connection::State::Failed) {
228228
FUERTE_LOG_ERROR << "queued request on failed connection\n";
229+
drainQueue(fuerte::Error::ConnectionClosed);
229230
}
230231
return mid;
231232
}

‎3rdParty/fuerte/src/VstConnection.cpp

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ MessageID VstConnection<ST>::sendRequest(std::unique_ptr<Request> req,
9191
this->startConnection();
9292
} else if (state == Connection::State::Failed) {
9393
FUERTE_LOG_ERROR << "queued request on failed connection\n";
94+
drainQueue(fuerte::Error::ConnectionClosed);
9495
}
9596
return mid;
9697
}
@@ -118,15 +119,13 @@ void VstConnection<ST>::finishConnect() {
118119
auto self = Connection::shared_from_this();
119120
asio_ns::async_write(
120121
this->_protocol.socket, asio_ns::buffer(vstHeader, strlen(vstHeader)),
121-
[self](asio_ns::error_code const& ec, std::size_t transferred) {
122+
[self](asio_ns::error_code const& ec, std::size_t nsend) {
122123
auto* thisPtr = static_cast<VstConnection<ST>*>(self.get());
123124
if (ec) {
124125
FUERTE_LOG_ERROR << ec.message() << "\n";
125-
thisPtr->shutdownConnection(Error::CouldNotConnect);
126+
thisPtr->shutdownConnection(Error::CouldNotConnect,
127+
"unable to connect: " + ec.message());
126128
thisPtr->drainQueue(Error::CouldNotConnect);
127-
thisPtr->onFailure(
128-
Error::CouldNotConnect,
129-
"unable to initialize connection: error=" + ec.message());
130129
return;
131130
}
132131
FUERTE_LOG_CALLBACKS << "VST connection established\n";
@@ -150,22 +149,23 @@ void VstConnection<ST>::sendAuthenticationRequest() {
150149
auto item = std::make_shared<RequestItem>();
151150
item->_messageID = vstMessageId.fetch_add(1, std::memory_order_relaxed);
152151
item->_expires = std::chrono::steady_clock::now() + Request::defaultTimeout;
153-
item->_request = nullptr; // should not break anything
154-
155152
auto self = Connection::shared_from_this();
156153
item->_callback = [self](Error error, std::unique_ptr<Request>,
157154
std::unique_ptr<Response> resp) {
155+
auto* thisPtr = static_cast<VstConnection<ST>*>(self.get());
158156
if (error != Error::NoError || resp->statusCode() != StatusOK) {
159-
auto* thisPtr = static_cast<VstConnection<ST>*>(self.get());
160157
thisPtr->_state.store(Connection::State::Failed,
161158
std::memory_order_release);
162-
thisPtr->shutdownConnection(Error::CouldNotConnect);
163-
thisPtr->onFailure(error, "authentication failed");
159+
thisPtr->shutdownConnection(Error::VstUnauthorized,
160+
"could not authenticate");
161+
thisPtr->drainQueue(Error::VstUnauthorized);
162+
} else {
163+
thisPtr->_state.store(Connection::State::Connected);
164+
thisPtr->startWriting();
164165
}
165166
};
166167

167168
_messageStore.add(item); // add message to store
168-
setTimeout(); // set request timeout
169169

170170
if (this->_config._authenticationType == AuthenticationType::Basic) {
171171
vst::message::authBasic(this->_config._user, this->_config._password,
@@ -176,25 +176,23 @@ void VstConnection<ST>::sendAuthenticationRequest() {
176176
assert(item->_buffer.size() < defaultMaxChunkSize);
177177

178178
// actually send auth request
179-
asio_ns::post(*this->_io_context, [this, self, item] {
180-
auto cb = [self, item, this](asio_ns::error_code const& ec,
181-
std::size_t transferred) {
182-
if (ec) {
183-
asyncWriteCallback(ec, transferred, std::move(item)); // error handling
184-
return;
185-
}
186-
this->_state.store(Connection::State::Connected,
187-
std::memory_order_release);
188-
asyncWriteCallback(ec, transferred,
189-
std::move(item)); // calls startReading()
190-
startWriting(); // start writing if something was queued
191-
};
192-
std::vector<asio_ns::const_buffer> buffers;
193-
vst::message::prepareForNetwork(
194-
_vstVersion, item->messageID(), item->_buffer,
195-
/*payload*/ asio_ns::const_buffer(), buffers);
196-
asio_ns::async_write(this->_protocol.socket, buffers, std::move(cb));
197-
});
179+
auto cb = [this, self, item](asio_ns::error_code const& ec,
180+
std::size_t nsend) {
181+
if (ec) {
182+
this->_state.store(Connection::State::Failed);
183+
this-> shutdownConnection(Error::CouldNotConnect,
184+
"authorization message failed");
185+
this->drainQueue(Error::CouldNotConnect);
186+
} else {
187+
asyncWriteCallback(ec, nsend, item);
188+
}
189+
};
190+
std::vector<asio_ns::const_buffer> buffers;
191+
vst::message::prepareForNetwork(_vstVersion, item->messageID(), item->_buffer,
192+
/*payload*/ asio_ns::const_buffer(), buffers);
193+
asio_ns::async_write(this->_protocol.socket, buffers, std::move(cb));
194+
195+
setTimeout();
198196
}
199197

200198
// ------------------------------------
@@ -268,7 +266,7 @@ void VstConnection<ST>::asyncWriteNextRequest() {
268266
// callback of async_write function that is called in sendNextRequest.
269267
template <SocketType ST>
270268
void VstConnection<ST>::asyncWriteCallback(asio_ns::error_code const& ec,
271-
std::size_t transferred,
269+
std::size_t nsend,
272270
std::shared_ptr<RequestItem> item) {
273271
// auto pendingAsyncCalls = --_connection->_async_calls;
274272
if (ec) {
@@ -288,7 +286,7 @@ void VstConnection<ST>::asyncWriteCallback(asio_ns::error_code const& ec,
288286
}
289287
// Send succeeded
290288
FUERTE_LOG_CALLBACKS << "asyncWriteCallback (vst): send succeeded, "
291-
<< transferred << " bytes transferred\n";
289+
<< nsend << " bytes send\n";
292290

293291
// request is written we no longer need data for that
294292
item->resetSendData();
@@ -389,8 +387,8 @@ void VstConnection<ST>::asyncReadCallback(asio_ns::error_code const& ec) {
389387
if (parser::ChunkState::Incomplete == state) {
390388
break;
391389
} else if (parser::ChunkState::Invalid == state) {
392-
FUERTE_LOG_ERROR << "Invalid VST chunk";
393-
this->shutdownConnection(Error::ProtocolError);
390+
this->shutdownConnection(Error::ProtocolError,
391+
"Invalid VST chunk");
394392
return;
395393
}
396394

@@ -496,12 +494,12 @@ std::unique_ptr<fu::Response> VstConnection<ST>::createResponse(
496494
// adjust the timeouts (only call from IO-Thread)
497495
template <SocketType ST>
498496
void VstConnection<ST>::setTimeout() {
499-
asio_ns::error_code ec;
500-
this->_timeout.cancel(ec);
501-
if (ec) {
502-
FUERTE_LOG_ERROR << "error on timeout cancel: " << ec.message();
503-
return; // bail out
504-
}
497+
// asio_ns::error_code ec;
498+
// this->_timeout.cancel(ec);
499+
// if (ec) {
500+
// FUERTE_LOG_ERROR << "error on timeout cancel: " << ec.message();
501+
// return; // bail out
502+
// }
505503

506504
// set to smallest point in time
507505
auto expires = std::chrono::steady_clock::time_point::max();

‎3rdParty/fuerte/src/types.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,9 @@ std::string to_string(Error error) {
234234
return "Error while writing ";
235235
case Error::Canceled:
236236
return "Connection was locally canceled";
237+
238+
case Error::VstUnauthorized:
239+
return "Cannot authorize on VST connection";
237240

238241
case Error::ProtocolError:
239242
return "Error: invalid server response";

‎arangod/Network/Utils.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ int fuerteToArangoErrorCode(fuerte::Error err) {
204204
case fuerte::Error::Canceled:
205205
case fuerte::Error::ProtocolError:
206206
return TRI_ERROR_CLUSTER_CONNECTION_LOST;
207+
208+
case fuerte::Error::VstUnauthorized:
209+
return TRI_ERROR_FORBIDDEN;
207210
}
208211

209212
return TRI_ERROR_INTERNAL;

0 commit comments

Comments
 (0)
0