Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transfer-encoding in reverse order #721

Merged
merged 4 commits into from
Oct 23, 2020
Merged

Fix transfer-encoding in reverse order #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
defc807
cohttp.headers: transfer-encoding is in reverse order
mseri Oct 22, 2020
42438c4
Update header tests, add test for 695
mseri Oct 22, 2020
c581b1d
Minor test cleanup
mseri Oct 22, 2020
a6dd439
Update CHANGES
mseri Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
- cohttp: fix chunked encoding of empty body (@mefyl #715)
- cohttp-async: fix body not being uploaded with unchunked Async.Pipe (@mefyl #706)
- cohttp-{async, lwt}: fix suprising behaviours of Body.is_empty (@anuragsoni #714 #712 #713)
- port to conduit 3.0.0 and documentation update, includes minor breaking changes on the API and the distinction between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl) (@dinosaure #692 #716)
- port to conduit 3.0.0: minor breaking changes on the server API, an explicit distinction
between cohttp-lwt-unix (using tls), cohttp-lwt-notls and cohttp-lwt-ssl (using lwt_ssl),
and includes ssl in cohttp-async (@dinosaure #692)
- cohttp-lwt-jsoo: rename Cohttp_lwt_xhr to Cohttp_lwt_jsoo for consistency (@mseri #717)
- refactoring of tests (@mseri #709, @dinosaure #692)
- update documentation (@dinosaure #716, @mseri #720)
- cohttp: fix transfer-encoding ordering in headers (@mseri #721)

## v2.5.4 (2020-07-21)

Expand Down
13 changes: 11 additions & 2 deletions cohttp/src/header.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ let headers_with_list_values = Array.map LString.of_string [|
"proxy-authenticate";"te";"trailer";"transfer-encoding";"upgrade";
"vary";"via";"warning";"www-authenticate"; |]

let is_transfer_encoding =
let k = LString.of_string "transfer-encoding" in
fun k' -> LString.compare k k' = 0

let is_header_with_list_value =
let tbl = Hashtbl.create (Array.length headers_with_list_values) in
headers_with_list_values |> Array.iter (fun h -> Hashtbl.add tbl h ());
Expand All @@ -57,7 +61,11 @@ let init_with k v =

let add h k v =
let k = LString.of_string k in
try StringMap.add k (v::(StringMap.find k h)) h
try
if is_transfer_encoding k then
StringMap.add k ((StringMap.find k h) @ [v]) h
else
StringMap.add k (v::(StringMap.find k h)) h
with Not_found -> StringMap.add k [v] h

let add_list h l =
Expand Down Expand Up @@ -122,7 +130,8 @@ let iter fn h = ignore(map fn h)
let fold fn h acc = StringMap.fold
(fun k v acc -> List.fold_left (fun acc v -> fn (LString.to_string k) v acc) acc v)
h acc
let of_list l = List.fold_left (fun h (k,v) -> add h k v) (init ()) l
let of_list l =
List.fold_left (fun h (k,v) -> add h k v) (init ()) l

let to_list h = List.rev (fold (fun k v acc -> (k,v)::acc) h [])
let header_line k v = Printf.sprintf "%s: %s\r\n" k v
Expand Down
3 changes: 2 additions & 1 deletion cohttp/test/test_body.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ let test_if_body_empty () =
Alcotest.(check bool) name (Cohttp.Body.is_empty body) expected
) tests

let () = Printexc.record_backtrace true

let () =
Printexc.record_backtrace true;
Alcotest.run "test_body" [
"Query body information", [
"Check if body is empty", `Quick, test_if_body_empty;
Expand Down
119 changes: 65 additions & 54 deletions cohttp/test/test_header.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,58 +485,69 @@ let test_cachecontrol_concat () =
aeso "test_cachecontrol_concat"
(Some "public,max-age:86400") (H.get h "Cache-Control")

;;
Printexc.record_backtrace true;
Alcotest.run "test_header" [
"Link", [
"simple", `Quick, link_simple;
"multiple rels", `Quick, link_multi_rel;
"multiple lines", `Quick, link_multi_line;
"multiheader", `Quick, link_multi_multi;
"rel uri", `Quick, link_rel_uri;
"anchor", `Quick, link_anchor;
"rev", `Quick, link_rev;
"media", `Quick, link_media;
"media complex", `Quick, link_media_complex;
"title", `Quick, link_title;
"title star", `Quick, link_title_star;
"type token", `Quick, link_type_token;
"type quoted", `Quick, link_type_quoted;
"extension", `Quick, link_ext;
"extension star", `Quick, link_ext_star;
];
"Media Type", [
"Media Type", `Quick, get_media_type;
];
"Auth", [
"Valid Auth", `Quick, valid_auth;
];
"Cookie", [
"Valid Set-Cookie", `Quick, valid_set_cookie;
"Valid Cookie", `Quick, valid_cookie;
"Cookie with =", `Quick, cookie_with_eq_val;
"Ignores empty cookie", `Quick, ignores_empty_cookie;
];
"Content Range", [
"none", `Quick, Content_range.none;
"content-length", `Quick, Content_range.content_length;
"content-range", `Quick, Content_range.content_range;
];
"Cache Control", [
"concat", `Quick, test_cachecontrol_concat
];
"Header", [
"get list valued", `Quick, list_valued_header;
"trim whitespace", `Quick, trim_ws;
"replace existing", `Quick, Updates.replace_headers_if_exists;
"replace absent", `Quick, Updates.replace_headers_if_absent;
"update existing", `Quick, Updates.update_headers_if_exists;
"update existing list", `Quick, Updates.update_headers_if_exists_multi;
"update add absent", `Quick, Updates.update_headers_if_absent_add;
"update rm existing", `Quick, Updates.update_headers_if_exists_rm;
"update rm absent", `Quick, Updates.update_headers_if_absent_rm;
"update absent", `Quick, Updates.update_headers_if_absent;
"many headers", `Slow, many_headers;
let transfer_encoding () =
let h = H.of_list ["transfer-encoding", "gzip";
"transfer-encoding", "chunked"] in
let sh = H.to_string h in
aes "transfer_encoding_string_is_ordered"
sh "transfer-encoding: gzip\r\ntransfer-encoding: chunked\r\n\r\n";
let sh = H.get h "transfer-encoding" in
aeso "transfer_encoding_get_is_ordered" (Some "gzip,chunked") sh

let () = Printexc.record_backtrace true

let () =
Alcotest.run "test_header" [
"Link", [
"simple", `Quick, link_simple;
"multiple rels", `Quick, link_multi_rel;
"multiple lines", `Quick, link_multi_line;
"multiheader", `Quick, link_multi_multi;
"rel uri", `Quick, link_rel_uri;
"anchor", `Quick, link_anchor;
"rev", `Quick, link_rev;
"media", `Quick, link_media;
"media complex", `Quick, link_media_complex;
"title", `Quick, link_title;
"title star", `Quick, link_title_star;
"type token", `Quick, link_type_token;
"type quoted", `Quick, link_type_quoted;
"extension", `Quick, link_ext;
"extension star", `Quick, link_ext_star;
];
"Media Type", [
"Media Type", `Quick, get_media_type;
];
"Auth", [
"Valid Auth", `Quick, valid_auth;
];
"Cookie", [
"Valid Set-Cookie", `Quick, valid_set_cookie;
"Valid Cookie", `Quick, valid_cookie;
"Cookie with =", `Quick, cookie_with_eq_val;
"Ignores empty cookie", `Quick, ignores_empty_cookie;
];
"Content Range", [
"none", `Quick, Content_range.none;
"content-length", `Quick, Content_range.content_length;
"content-range", `Quick, Content_range.content_range;
];
"Cache Control", [
"concat", `Quick, test_cachecontrol_concat
];
"Header", [
"get list valued", `Quick, list_valued_header;
"trim whitespace", `Quick, trim_ws;
"replace existing", `Quick, Updates.replace_headers_if_exists;
"replace absent", `Quick, Updates.replace_headers_if_absent;
"update existing", `Quick, Updates.update_headers_if_exists;
"update existing list", `Quick, Updates.update_headers_if_exists_multi;
"update add absent", `Quick, Updates.update_headers_if_absent_add;
"update rm existing", `Quick, Updates.update_headers_if_exists_rm;
"update rm absent", `Quick, Updates.update_headers_if_absent_rm;
"update absent", `Quick, Updates.update_headers_if_absent;
"many headers", `Slow, many_headers;
"transfer encoding is in correct order", `Quick, transfer_encoding;
]
@ if Sys.word_size = 64 then ["large header", `Slow, large_header] else []
]
@ if Sys.word_size = 64 then ["large header", `Slow, large_header] else []
]
93 changes: 47 additions & 46 deletions cohttp/test/test_request.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,49 +249,50 @@ let uri_round_trip _ =
let actual_uri = Request.make expected_uri |> Request.uri in
Alcotest.check uri_testable "Request.make uri round-trip" actual_uri expected_uri

;;
Printexc.record_backtrace true;
Alcotest.run "test_request" [
"Auth", [
"header has auth", `Quick, header_has_auth;
"URI has user info", `Quick, uri_has_userinfo;
"from URI - do not override", `Quick, auth_uri_no_override;
"from URI", `Quick, auth_uri;
];
"Encoding", [
"from content-length header",`Quick, encoding_content_length_header;
"from transfer-encoding header",`Quick, encoding_transfer_encoding_header;
"with both headers",`Quick, encoding_both_headers;
"from both optional argument and headers", `Quick, encoding_header_opt_argument;
];
"Parse URI", [
"simple", `Quick, parse_request_uri;
"with host", `Quick, parse_request_uri_host;
"with host and port", `Quick, parse_request_uri_host_port;
"double slash", `Quick, parse_request_uri_double_slash;
"double slash with host", `Quick, parse_request_uri_host_double_slash;
"triple slash", `Quick, parse_request_uri_triple_slash;
"triple slash with host", `Quick, parse_request_uri_host_triple_slash;
"no slash", `Quick, parse_request_uri_no_slash;
"no slash with host", `Quick, parse_request_uri_host_no_slash;
"empty", `Quick, parse_request_uri_empty;
"empty with host", `Quick, parse_request_uri_host_empty;
"path like scheme", `Quick, parse_request_uri_path_like_scheme;
"path like scheme with host", `Quick, parse_request_uri_host_path_like_scheme;
"path like host:port", `Quick, parse_request_uri_path_like_host_port;
"path like host:port with host",
`Quick, parse_request_uri_host_path_like_host_port;
"with query string", `Quick, parse_request_uri_query;
"with query with host", `Quick, parse_request_uri_host_query;
"no slash with query string", `Quick, parse_request_uri_query_no_slash;
"no slash with query with host",
`Quick, parse_request_uri_host_query_no_slash;
"CONNECT", `Quick, parse_request_connect;
"CONNECT with host", `Quick, parse_request_connect_host;
"OPTIONS", `Quick, parse_request_options;
"OPTIONS with host", `Quick, parse_request_options_host;
"parent traversal", `Quick, parse_request_uri_traversal;
"parent traversal with host", `Quick, parse_request_uri_host_traversal;
"uri round-trip", `Quick, uri_round_trip;
];
]
let () = Printexc.record_backtrace true

let () =
Alcotest.run "test_request" [
"Auth", [
"header has auth", `Quick, header_has_auth;
"URI has user info", `Quick, uri_has_userinfo;
"from URI - do not override", `Quick, auth_uri_no_override;
"from URI", `Quick, auth_uri;
];
"Encoding", [
"from content-length header",`Quick, encoding_content_length_header;
"from transfer-encoding header",`Quick, encoding_transfer_encoding_header;
"with both headers",`Quick, encoding_both_headers;
"from both optional argument and headers", `Quick, encoding_header_opt_argument;
];
"Parse URI", [
"simple", `Quick, parse_request_uri;
"with host", `Quick, parse_request_uri_host;
"with host and port", `Quick, parse_request_uri_host_port;
"double slash", `Quick, parse_request_uri_double_slash;
"double slash with host", `Quick, parse_request_uri_host_double_slash;
"triple slash", `Quick, parse_request_uri_triple_slash;
"triple slash with host", `Quick, parse_request_uri_host_triple_slash;
"no slash", `Quick, parse_request_uri_no_slash;
"no slash with host", `Quick, parse_request_uri_host_no_slash;
"empty", `Quick, parse_request_uri_empty;
"empty with host", `Quick, parse_request_uri_host_empty;
"path like scheme", `Quick, parse_request_uri_path_like_scheme;
"path like scheme with host", `Quick, parse_request_uri_host_path_like_scheme;
"path like host:port", `Quick, parse_request_uri_path_like_host_port;
"path like host:port with host",
`Quick, parse_request_uri_host_path_like_host_port;
"with query string", `Quick, parse_request_uri_query;
"with query with host", `Quick, parse_request_uri_host_query;
"no slash with query string", `Quick, parse_request_uri_query_no_slash;
"no slash with query with host",
`Quick, parse_request_uri_host_query_no_slash;
"CONNECT", `Quick, parse_request_connect;
"CONNECT with host", `Quick, parse_request_connect_host;
"OPTIONS", `Quick, parse_request_options;
"OPTIONS with host", `Quick, parse_request_options_host;
"parent traversal", `Quick, parse_request_uri_traversal;
"parent traversal with host", `Quick, parse_request_uri_host_traversal;
"uri round-trip", `Quick, uri_round_trip;
];
]