Skip to content

Commit 34afd97

Browse files
awwitawwit
committed
Fixed bugs in the HTTP/2 protocol
1 parent c25275d commit 34afd97

File tree

11 files changed

+155
-40
lines changed

11 files changed

+155
-40
lines changed

‎src/server/protocol/ServerHttp2.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -478,16 +478,16 @@ namespace HttpServer
478478

479479
static bool getNextHttp2FrameMeta(const Socket::Adapter &sock, const std::chrono::milliseconds &timeout, std::vector<char> &buf, Http2::FrameMeta &meta, long &read_size)
480480
{
481-
if (read_size <= meta.length + Http2::FRAME_HEADER_SIZE)
481+
if (read_size <= static_cast<long>(meta.length + Http2::FRAME_HEADER_SIZE) )
482482
{
483-
if (read_size == meta.length + Http2::FRAME_HEADER_SIZE)
483+
if (read_size == static_cast<long>(meta.length + Http2::FRAME_HEADER_SIZE) )
484484
{
485485
read_size = 0;
486486
}
487487

488488
read_size = sock.nonblock_recv(buf.data() + read_size, buf.size() - read_size, timeout);
489489

490-
if (read_size < Http2::FRAME_HEADER_SIZE)
490+
if (read_size < static_cast<long>(Http2::FRAME_HEADER_SIZE) )
491491
{
492492
return false;
493493
}
@@ -496,7 +496,7 @@ namespace HttpServer
496496
{
497497
std::copy(buf.cbegin() + meta.length + Http2::FRAME_HEADER_SIZE, buf.cbegin() + read_size, buf.begin() );
498498

499-
read_size -= meta.length + Http2::FRAME_HEADER_SIZE;
499+
read_size -= static_cast<long>(meta.length + Http2::FRAME_HEADER_SIZE);
500500
}
501501

502502
const uint8_t *addr = reinterpret_cast<const uint8_t *>(buf.data() );

‎src/server/protocol/ServerHttp2Protocol.cpp‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,11 @@ namespace HttpServer
3838

3939
std::vector<char> buf;
4040
buf.reserve(4096);
41+
buf.resize(Http2::FRAME_HEADER_SIZE);
4142

4243
HPack::pack(buf, headers, this->stream->conn.encoding_dynamic_table);
4344

44-
const uint32_t frame_size = buf.size();
45-
46-
buf.insert(buf.begin(), Http2::FRAME_HEADER_SIZE, 0);
45+
const uint32_t frame_size = buf.size() - Http2::FRAME_HEADER_SIZE;
4746

4847
Http2::FrameFlag flags = Http2::FrameFlag::END_HEADERS;
4948

@@ -54,7 +53,16 @@ namespace HttpServer
5453

5554
this->stream->setHttp2FrameHeader(reinterpret_cast<uint8_t *>(buf.data() ), frame_size, Http2::FrameType::HEADERS, flags);
5655

57-
return this->sock.nonblock_send(buf.data(), buf.size(), timeout) > 0; // >= 0
56+
this->stream->lock();
57+
58+
auto const is_sended = this->sock.nonblock_send(buf.data(), buf.size(), timeout) > 0; // >= 0;
59+
60+
if (endStream || false == is_sended)
61+
{
62+
this->stream->unlock();
63+
}
64+
65+
return is_sended;
5866
}
5967

6068
long ServerHttp2Protocol::sendData(const void *src, size_t size, const std::chrono::milliseconds &timeout, DataTransfer *dt) const
@@ -70,8 +78,6 @@ namespace HttpServer
7078

7179
while (size != 0)
7280
{
73-
buf.resize(0);
74-
7581
// TODO: test with data_size == 1 (padding length == 0)
7682
size_t data_size = setting.max_frame_size < size ? setting.max_frame_size : size;
7783

@@ -88,6 +94,8 @@ namespace HttpServer
8894

8995
const size_t frame_size = data_size + padding_size;
9096

97+
buf.resize(frame_size + Http2::FRAME_HEADER_SIZE);
98+
9199
/* if (this->stream->window_size_out - frame_size <= 0)
92100
{
93101
size_t update_size = (dt->full_size - dt->send_total) - this->stream->window_size_out;
@@ -109,30 +117,33 @@ namespace HttpServer
109117
flags |= Http2::FrameFlag::END_STREAM;
110118
}
111119

120+
size_t cur = Http2::FRAME_HEADER_SIZE;
121+
112122
if (padding_size)
113123
{
114124
flags |= Http2::FrameFlag::PADDED;
115125

116-
buf.insert(buf.begin(), sizeof(uint8_t), padding);
117-
}
126+
buf[cur] = padding;
118127

119-
buf.insert(buf.begin(), Http2::FRAME_HEADER_SIZE, 0);
128+
++cur;
129+
}
120130

121131
const Http2::FrameType frame_type = Http2::FrameType::DATA;
122132

123133
this->stream->setHttp2FrameHeader(buf.data(), frame_size, frame_type, flags);
124134

125-
std::copy(data, data + data_size, std::back_inserter(buf) );
135+
std::copy(data, data + data_size, buf.begin() + cur);
126136

127137
if (padding)
128138
{
129-
buf.insert(buf.end(), padding, 0);
139+
std::fill(buf.end() - padding, buf.end(), 0);
130140
}
131141

132142
long sended = this->sock.nonblock_send(buf.data(), buf.size(), timeout);
133143

134144
if (sended <= 0)
135145
{
146+
send_size = sended;
136147
break;
137148
}
138149

@@ -144,6 +155,11 @@ namespace HttpServer
144155
size -= data_size;
145156
}
146157

158+
if (0 >= send_size || dt->full_size == dt->send_total)
159+
{
160+
this->stream->unlock();
161+
}
162+
147163
return send_size;
148164
}
149165

@@ -163,6 +179,7 @@ namespace HttpServer
163179
Utils::packNumber(buf, this->stream->conn.client_settings.max_frame_size);
164180
Utils::packNumber(buf, this->stream->conn.client_settings.max_header_list_size);
165181
Utils::packContainer(buf, this->stream->conn.encoding_dynamic_table.getList() );
182+
Utils::packPointer(buf, &this->stream->conn.sync.mtx);
166183
Utils::packContainer(buf, req.incoming_headers);
167184
Utils::packContainer(buf, req.incoming_data);
168185
Utils::packFilesIncoming(buf, req.incoming_files);

‎src/socket/AdapterTls.cpp‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,25 +63,36 @@ namespace Socket
6363

6464
long AdapterTls::nonblock_send_all(const void *buf, const size_t length, const std::chrono::milliseconds &timeout) const noexcept
6565
{
66-
size_t record_size = ::gnutls_record_get_max_size(this->session);
66+
// size_t record_size = ::gnutls_record_get_max_size(this->session);
67+
size_t record_size = length;
6768

6869
if (0 == record_size)
6970
{
7071
return -1;
7172
}
7273

74+
Socket sock(this->get_handle() );
75+
76+
// ::gnutls_record_set_timeout(this->session, static_cast<unsigned int>(timeout.count() ) );
77+
7378
size_t total = 0;
7479

7580
while (total < length)
7681
{
77-
::gnutls_record_set_timeout(this->session, static_cast<unsigned int>(timeout.count() ) );
78-
7982
if (record_size > length - total)
8083
{
8184
record_size = length - total;
8285
}
8386

84-
const long send_size = ::gnutls_record_send(this->session, reinterpret_cast<const uint8_t *>(buf) + total, record_size);
87+
// const long send_size = ::gnutls_record_send(this->session, reinterpret_cast<const uint8_t *>(buf) + total, record_size);
88+
89+
long send_size = 0;
90+
91+
do
92+
{
93+
sock.nonblock_send_sync();
94+
}
95+
while (GNUTLS_E_AGAIN == (send_size = ::gnutls_record_send(this->session, reinterpret_cast<const uint8_t *>(buf) + total, record_size) ) );
8596

8697
if (send_size < 0)
8798
{
@@ -111,7 +122,11 @@ namespace Socket
111122

112123
long AdapterTls::nonblock_recv(void *buf, const size_t length, const std::chrono::milliseconds &timeout) const noexcept
113124
{
114-
::gnutls_record_set_timeout(this->session, static_cast<const unsigned int>(timeout.count() ) );
125+
// ::gnutls_record_set_timeout(this->session, static_cast<const unsigned int>(timeout.count() ) );
126+
127+
Socket sock(this->get_handle() );
128+
sock.nonblock_recv_sync();
129+
115130
return ::gnutls_record_recv(this->session, buf, length);
116131
}
117132

‎src/socket/List.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ namespace Socket
265265

266266
do
267267
{
268-
struct ::sockaddr_in client_addr = {};
268+
::sockaddr_in client_addr {};
269269
socklen_t client_addr_len = sizeof(client_addr);
270270

271271
client_socket = ::accept(event.fd, reinterpret_cast<::sockaddr *>(&client_addr), &client_addr_len);
@@ -299,7 +299,7 @@ namespace Socket
299299

300300
do
301301
{
302-
struct ::sockaddr_in client_addr = {};
302+
::sockaddr_in client_addr {};
303303
socklen_t client_addr_len = sizeof(client_addr);
304304

305305
client_socket = ::accept(event.data.fd, reinterpret_cast<::sockaddr *>(&client_addr), &client_addr_len);

‎src/socket/Socket.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,29 @@ namespace Socket
314314
return recv_len;
315315
}
316316

317+
void Socket::nonblock_recv_sync() const noexcept
318+
{
319+
#ifdef WIN32
320+
WSAPOLLFD event = {
321+
this->socket_handle,
322+
POLLRDNORM | POLLRDBAND,
323+
0
324+
};
325+
326+
::WSAPoll(&event, 1, ~0);
327+
#elif POSIX
328+
struct ::pollfd event = {
329+
this->socket_handle,
330+
POLLIN,
331+
0
332+
};
333+
334+
::poll(&event, 1, ~0);
335+
#else
336+
#error "Undefine platform"
337+
#endif
338+
}
339+
317340
static long send_all(const System::native_socket_type socket_handle, const void *data, const size_t length) noexcept
318341
{
319342
size_t total = 0;

‎src/socket/Socket.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ namespace Socket
5151
long nonblock_recv(std::vector<std::string::value_type> &buf, const std::chrono::milliseconds &timeout) const noexcept;
5252
long nonblock_recv(void *buf, const size_t length, const std::chrono::milliseconds &timeout) const noexcept;
5353

54+
void nonblock_recv_sync() const noexcept;
55+
5456
long send(const std::string &buf) const noexcept;
5557
long send(const void *buf, const size_t length) const noexcept;
5658

‎src/system/System.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ namespace System
228228

229229
::FileTimeToSystemTime(&ftWrite, &stUtc);
230230

231-
struct ::tm tm_time {
231+
std::tm tm_time {
232232
stUtc.wSecond,
233233
stUtc.wMinute,
234234
stUtc.wHour,
@@ -240,11 +240,11 @@ namespace System
240240
-1
241241
};
242242

243-
*fileTime = ::mktime(&tm_time);
243+
*fileTime = std::mktime(&tm_time);
244244

245245
return true;
246246
#elif POSIX
247-
struct ::stat attrib;
247+
struct ::stat attrib {};
248248

249249
if (-1 == ::stat(filePath.c_str(), &attrib) )
250250
{
@@ -253,11 +253,11 @@ namespace System
253253

254254
*fileSize = attrib.st_size;
255255

256-
struct ::tm clock = {};
256+
std::tm clock {};
257257

258258
::gmtime_r(&(attrib.st_mtime), &clock);
259259

260-
*fileTime = ::mktime(&clock);
260+
*fileTime = std::mktime(&clock);
261261

262262
return true;
263263
#else
@@ -284,7 +284,7 @@ namespace System
284284
memory_name.erase(memory_name.begin() + pos, memory_name.end() );
285285
}
286286

287-
::TCHAR buf[MAX_PATH + 1] = {};
287+
::TCHAR buf[MAX_PATH + 1] {};
288288
::GetFullPathName(memory_name.c_str(), MAX_PATH, buf, nullptr);
289289

290290
#ifdef UNICODE

‎src/transfer/http2/Http2.cpp‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,16 @@ namespace Http2
144144
return (addr + Http2::FRAME_HEADER_SIZE);
145145
}
146146

147+
void IncStream::lock()
148+
{
149+
this->conn.sync.mtx.lock();
150+
}
151+
152+
void IncStream::unlock() noexcept
153+
{
154+
this->conn.sync.mtx.unlock();
155+
}
156+
147157
void IncStream::close() noexcept
148158
{
149159
this->incoming_headers.clear();
@@ -157,15 +167,15 @@ namespace Http2
157167
// this->state = StreamState::CLOSED;
158168
}
159169

160-
OutStream::OutStream(const uint32_t streamId, const ConnectionSettings &settings, DynamicTable &&dynamic_table) noexcept
161-
: stream_id(streamId), settings(settings), window_size_out(settings.initial_window_size), dynamic_table(std::move(dynamic_table) )
170+
OutStream::OutStream(const uint32_t streamId, const ConnectionSettings &settings, DynamicTable &&dynamic_table, std::mutex *mtx) noexcept
171+
: stream_id(streamId), settings(settings), window_size_out(settings.initial_window_size), dynamic_table(std::move(dynamic_table) ), mtx(mtx)
162172
{
163173

164174
}
165175

166-
OutStream::OutStream(const IncStream &stream) noexcept
176+
OutStream::OutStream(const IncStream &stream)
167177
: stream_id(stream.stream_id), settings(stream.conn.client_settings),
168-
window_size_out(stream.window_size_out), dynamic_table(stream.conn.encoding_dynamic_table)
178+
window_size_out(stream.window_size_out), dynamic_table(stream.conn.encoding_dynamic_table), mtx(&stream.conn.sync.mtx)
169179
{
170180

171181
}
@@ -179,4 +189,14 @@ namespace Http2
179189

180190
return (addr + Http2::FRAME_HEADER_SIZE);
181191
}
192+
193+
void OutStream::lock()
194+
{
195+
this->mtx->lock();
196+
}
197+
198+
void OutStream::unlock() noexcept
199+
{
200+
this->mtx->unlock();
201+
}
182202
};

‎src/transfer/http2/Http2.h‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ namespace Http2
173173

174174
uint8_t *setHttp2FrameHeader(uint8_t *addr, const uint32_t frameSize, const Http2::FrameType frameType, const Http2::FrameFlag frameFlags) noexcept;
175175

176+
void lock();
177+
void unlock() noexcept;
178+
176179
void close() noexcept;
177180
};
178181

@@ -186,10 +189,15 @@ namespace Http2
186189

187190
DynamicTable dynamic_table;
188191

192+
std::mutex *mtx;
193+
189194
public:
190-
OutStream(const uint32_t streamId, const ConnectionSettings &settings, DynamicTable &&dynamic_table) noexcept;
191-
OutStream(const IncStream &stream) noexcept;
195+
OutStream(const uint32_t streamId, const ConnectionSettings &settings, DynamicTable &&dynamic_table, std::mutex *mtx) noexcept;
196+
OutStream(const IncStream &stream);
192197

193198
uint8_t *setHttp2FrameHeader(uint8_t *addr, const uint32_t frameSize, const Http2::FrameType frameType, const Http2::FrameFlag frameFlags) noexcept;
199+
200+
void lock();
201+
void unlock() noexcept;
194202
};
195203
};

0 commit comments

Comments
 (0)