Skip to content

Commit a86c232

Browse files
kantaikantai
authored
Merge pull request stacks-network#5904 from stacks-network/fix/http-duplicate-headers
fix: http endpoints should tolerate duplicate headers stacks-network#5903
2 parents 2a29a43 + 1587527 commit a86c232

File tree

5 files changed

+84
-8
lines changed

5 files changed

+84
-8
lines changed

‎stackslib/src/net/api/tests/get_tenures_fork_info.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ fn make_preamble<T: Display, R: Display>(start: &T, stop: &R) -> HttpRequestPrea
3737
content_length: Some(0),
3838
keep_alive: false,
3939
headers: BTreeMap::new(),
40+
set_cookie: Vec::new(),
4041
}
4142
}
4243

‎stackslib/src/net/api/tests/getsigner.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ fn make_preamble(query: &str) -> HttpRequestPreamble {
4141
content_length: Some(0),
4242
keep_alive: false,
4343
headers: BTreeMap::new(),
44+
set_cookie: Vec::new(),
4445
}
4546
}
4647

‎stackslib/src/net/api/tests/getsortition.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ fn make_preamble(query: &str) -> HttpRequestPreamble {
4040
content_length: Some(0),
4141
keep_alive: false,
4242
headers: BTreeMap::new(),
43+
set_cookie: Vec::new(),
4344
}
4445
}
4546

‎stackslib/src/net/http/request.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
// You should have received a copy of the GNU General Public License
1515
// along with this program. If not, see <http://www.gnu.org/licenses/>.
1616

17+
use std::collections::btree_map::Entry;
1718
use std::collections::{BTreeMap, HashMap, HashSet};
19+
use std::fmt::Display;
1820
use std::io::{Read, Write};
1921

2022
use percent_encoding::percent_decode_str;
@@ -54,6 +56,8 @@ pub struct HttpRequestPreamble {
5456
pub keep_alive: bool,
5557
/// Other headers that were not consumed in parsing
5658
pub headers: BTreeMap<String, String>,
59+
/// `Set-Cookie` headers
60+
pub set_cookie: Vec<String>,
5761
}
5862

5963
impl HttpRequestPreamble {
@@ -74,6 +78,7 @@ impl HttpRequestPreamble {
7478
content_length: None,
7579
keep_alive,
7680
headers: BTreeMap::new(),
81+
set_cookie: vec![],
7782
}
7883
}
7984

@@ -105,6 +110,7 @@ impl HttpRequestPreamble {
105110
content_length: None,
106111
keep_alive: true,
107112
headers: BTreeMap::new(),
113+
set_cookie: vec![],
108114
}
109115
}
110116

@@ -187,10 +193,10 @@ impl HttpRequestPreamble {
187193
return Some(format!("{}", &self.host));
188194
}
189195
"content-type" => {
190-
return self.content_type.clone().map(|ct| format!("{}", &ct));
196+
return self.content_type.as_ref().map(HttpContentType::to_string);
191197
}
192198
"content-length" => {
193-
return self.content_length.clone().map(|cl| format!("{}", &cl));
199+
return self.content_length.as_ref().map(u32::to_string);
194200
}
195201
_ => {
196202
return self.headers.get(&hdr).cloned();
@@ -371,9 +377,10 @@ impl StacksMessageCodec for HttpRequestPreamble {
371377

372378
let mut headers: BTreeMap<String, String> = BTreeMap::new();
373379
let mut seen_headers: HashSet<String> = HashSet::new();
380+
let mut set_cookie = vec![];
374381

375-
for i in 0..req.headers.len() {
376-
let value = String::from_utf8(req.headers[i].value.to_vec()).map_err(|_e| {
382+
for req_header in req.headers.iter() {
383+
let value = String::from_utf8(req_header.value.to_vec()).map_err(|_e| {
377384
CodecError::DeserializeError(
378385
"Invalid HTTP header value: not utf-8".to_string(),
379386
)
@@ -389,31 +396,33 @@ impl StacksMessageCodec for HttpRequestPreamble {
389396
));
390397
}
391398

392-
let key = req.headers[i].name.to_string().to_lowercase();
399+
let key = req_header.name.to_lowercase();
393400

394401
if seen_headers.contains(&key) {
395402
return Err(CodecError::DeserializeError(format!(
396403
"Invalid HTTP request: duplicate header \"{}\"",
397404
key
398405
)));
399406
}
400-
seen_headers.insert(key.clone());
401407

402408
if key == "host" {
403409
peerhost = match value.parse::<PeerHost>() {
404410
Ok(ph) => Some(ph),
405411
Err(_) => None,
406412
};
413+
seen_headers.insert(key);
407414
} else if key == "content-type" {
408415
// parse
409416
let ctype = value.to_lowercase().parse::<HttpContentType>()?;
410417
content_type = Some(ctype);
418+
seen_headers.insert(key);
411419
} else if key == "content-length" {
412420
// parse
413421
content_length = match value.parse::<u32>() {
414422
Ok(len) => Some(len),
415423
Err(_) => None,
416424
};
425+
seen_headers.insert(key);
417426
} else if key == "connection" {
418427
// parse
419428
if value.to_lowercase() == "close" {
@@ -425,8 +434,17 @@ impl StacksMessageCodec for HttpRequestPreamble {
425434
"Inavlid HTTP request: invalid Connection: header".to_string(),
426435
));
427436
}
437+
seen_headers.insert(key);
438+
} else if key == "set-cookie" {
439+
set_cookie.push(value);
428440
} else {
429-
headers.insert(key, value);
441+
headers
442+
.entry(key)
443+
.and_modify(|entry| {
444+
entry.push_str(", ");
445+
entry.push_str(&value);
446+
})
447+
.or_insert(value);
430448
}
431449
}
432450

@@ -445,6 +463,7 @@ impl StacksMessageCodec for HttpRequestPreamble {
445463
content_length,
446464
keep_alive,
447465
headers,
466+
set_cookie,
448467
})
449468
}
450469
}

‎stackslib/src/net/http/tests.rs

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
// You should have received a copy of the GNU General Public License
1515
// along with this program. If not, see <http://www.gnu.org/licenses/>.
1616

17-
use stacks_common::codec::StacksMessageCodec;
17+
use std::collections::BTreeMap;
18+
19+
use stacks_common::codec::{Error as CodecError, StacksMessageCodec};
1820
use stacks_common::types::net::{PeerAddress, PeerHost};
1921

2022
use crate::net::http::common::{HTTP_PREAMBLE_MAX_ENCODED_SIZE, HTTP_PREAMBLE_MAX_NUM_HEADERS};
@@ -78,6 +80,58 @@ fn test_parse_reserved_header() {
7880
}
7981
}
8082

83+
#[test]
84+
fn parse_http_request_duplicate_headers() {
85+
let tests = vec![
86+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nCache-Control: no-cache\r\ncache-control: no-store\r\nConnection: close\r\n\r\n",
87+
Ok(BTreeMap::from([("cache-control".to_string(), "no-cache, no-store".to_string())]))),
88+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nCache-Control: no-store\r\ncache-control: no-cache\r\nConnection: close\r\n\r\n",
89+
Ok(BTreeMap::from([("cache-control".into(), "no-store, no-cache".into())]))),
90+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nHost: core2.blockstack.org\r\nConnection: close\r\n\r\n",
91+
Err(CodecError::DeserializeError("Invalid HTTP request: duplicate header \"host\"".into()))),
92+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nConnection: close\r\nConnection: keep-alive\r\n\r\n",
93+
Err(CodecError::DeserializeError("Invalid HTTP request: duplicate header \"connection\"".into()))),
94+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nContent-Type: application/json\r\nContent-Type: application/json\r\nConnection: close\r\n\r\n",
95+
Err(CodecError::DeserializeError("Invalid HTTP request: duplicate header \"content-type\"".into()))),
96+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nContent-Length: 10\r\nContent-length: 5\r\nConnection: close\r\n\r\n",
97+
Err(CodecError::DeserializeError("Invalid HTTP request: duplicate header \"content-length\"".into()))),
98+
];
99+
100+
for (data, expected) in tests.into_iter() {
101+
let result = HttpRequestPreamble::consensus_deserialize(&mut data.as_bytes());
102+
match result {
103+
Ok(req) => {
104+
let expected = expected.unwrap();
105+
assert_eq!(req.headers, expected);
106+
}
107+
Err(e) => {
108+
let expected = expected.unwrap_err();
109+
assert_eq!(format!("{expected:?}"), format!("{e:?}"));
110+
}
111+
}
112+
}
113+
}
114+
115+
#[test]
116+
fn parse_http_request_set_cookie() {
117+
let tests = vec![
118+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nConnection: close\r\n\r\n",
119+
vec![]),
120+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nset-Cookie: a1\r\nSet-Cookie: a2\r\nConnection: close\r\n\r\n",
121+
vec!["a1".to_string(), "a2".to_string()]),
122+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nset-Cookie: a2\r\nSet-Cookie: a1\r\nConnection: close\r\n\r\n",
123+
vec!["a2".to_string(), "a1".to_string()]),
124+
("POST asdf HTTP/1.1\r\nHost: core.blockstack.org\r\nset-Cookie: a1\r\nConnection: close\r\n\r\n",
125+
vec!["a1".to_string()]),
126+
];
127+
128+
for (data, expected) in tests.into_iter() {
129+
let req = HttpRequestPreamble::consensus_deserialize(&mut data.as_bytes())
130+
.expect("Should be able to parse the set-cookie requests");
131+
assert_eq!(req.set_cookie, expected);
132+
}
133+
}
134+
81135
#[test]
82136
fn test_parse_http_request_preamble_ok() {
83137
let tests = vec![

0 commit comments

Comments
 (0)