Skip to content

Commit a31fea8

Browse files
authored
cleanup ProxyProtocol and prevent memory leak (#12680)
* cleanup ProxyProtocol and prevent memory leak * cleanup headers
1 parent e959247 commit a31fea8

File tree

4 files changed

+352
-9
lines changed

4 files changed

+352
-9
lines changed

include/iocore/net/ProxyProtocol.h

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,23 @@ class ProxyProtocol
6464
: version(pp_ver), ip_family(family), src_addr(src), dst_addr(dst)
6565
{
6666
}
67-
~ProxyProtocol() { ats_free(additional_data); }
67+
ProxyProtocol(const ProxyProtocol &other)
68+
: version(other.version), ip_family(other.ip_family), src_addr(other.src_addr), dst_addr(other.dst_addr)
69+
{
70+
if (!other.additional_data.empty()) {
71+
set_additional_data(other.additional_data);
72+
}
73+
}
74+
ProxyProtocol(ProxyProtocol &&other)
75+
: version(other.version), ip_family(other.ip_family), src_addr(other.src_addr), dst_addr(other.dst_addr)
76+
{
77+
if (!other.additional_data.empty()) {
78+
set_additional_data(other.additional_data);
79+
}
80+
other.additional_data.clear();
81+
other.tlv.clear();
82+
}
83+
~ProxyProtocol() = default;
6884
int set_additional_data(std::string_view data);
6985
void set_ipv4_addrs(in_addr_t src_addr, uint16_t src_port, in_addr_t dst_addr, uint16_t dst_port);
7086
void set_ipv6_addrs(const in6_addr &src_addr, uint16_t src_port, const in6_addr &dst_addr, uint16_t dst_port);
@@ -78,8 +94,46 @@ class ProxyProtocol
7894
IpEndpoint dst_addr = {};
7995
std::unordered_map<uint8_t, std::string_view> tlv;
8096

97+
ProxyProtocol &
98+
operator=(const ProxyProtocol &other)
99+
{
100+
if (&other == this) {
101+
return *this;
102+
}
103+
version = other.version;
104+
ip_family = other.ip_family;
105+
src_addr = other.src_addr;
106+
dst_addr = other.dst_addr;
107+
if (!other.additional_data.empty()) {
108+
set_additional_data(other.additional_data);
109+
} else {
110+
additional_data.clear();
111+
tlv.clear();
112+
}
113+
return *this;
114+
}
115+
116+
ProxyProtocol &
117+
operator=(ProxyProtocol &&other)
118+
{
119+
version = other.version;
120+
ip_family = other.ip_family;
121+
src_addr = other.src_addr;
122+
dst_addr = other.dst_addr;
123+
124+
additional_data.clear();
125+
tlv.clear();
126+
127+
if (!other.additional_data.empty()) {
128+
set_additional_data(other.additional_data);
129+
}
130+
other.additional_data.clear();
131+
other.tlv.clear();
132+
return *this;
133+
}
134+
81135
private:
82-
char *additional_data = nullptr;
136+
std::string additional_data;
83137
};
84138

85139
const size_t PPv1_CONNECTION_HEADER_LEN_MAX = 108;

include/proxy/http/HttpTransact.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include <cstddef>
2727

28+
#include "iocore/net/ProxyProtocol.h"
2829
#include "tsutil/DbgCtl.h"
2930
#include "tscore/ink_assert.h"
3031
#include "tscore/ink_platform.h"
@@ -883,6 +884,9 @@ class HttpTransact
883884
delete[] ranges;
884885
ranges = nullptr;
885886
range_setup = RangeSetup_t::NONE;
887+
888+
// This avoids a potential leak since sometimes this class is not destructed (ClassAllocated via HttpSM)
889+
pp_info.~ProxyProtocol();
886890
return;
887891
}
888892

src/iocore/net/ProxyProtocol.cc

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -552,14 +552,9 @@ ProxyProtocol::set_additional_data(std::string_view data)
552552
{
553553
uint16_t len = data.length();
554554
Dbg(dbg_ctl_proxyprotocol_v2, "Parsing %d byte additional data", len);
555-
additional_data = static_cast<char *>(ats_malloc(len));
556-
if (additional_data == nullptr) {
557-
Dbg(dbg_ctl_proxyprotocol_v2, "Memory allocation failed");
558-
return -1;
559-
}
560-
data.copy(additional_data, len);
555+
additional_data.assign(data);
561556

562-
const char *p = additional_data;
557+
const char *p = additional_data.data();
563558
const char *end = p + len;
564559
while (p != end) {
565560
if (end - p < 3) {

0 commit comments

Comments
 (0)