net-cpp

Merge lp:~gary-wzl77/net-cpp/fix_1631846 into lp:net-cpp

  1. fix_1631846
  2. Merge into trunk
Proposed by Gary.Wang
Status: Merged
Approved by: Thomas Voß
Approved revision: 55
Merged at revision: 52
Proposed branch: lp:~gary-wzl77/net-cpp/fix_1631846
Merge into: lp:net-cpp
Diff against target: 86 lines (+44/-2)
4 files modified
CMakeLists.txt (+1/-1)
debian/changelog (+8/-0)
src/core/net/http/impl/curl/request.h (+1/-1)
tests/http_client_test.cpp (+34/-0)
To merge this branch: bzr merge lp:~gary-wzl77/net-cpp/fix_1631846
Reviewer Review Type Date Requested Status
Thomas Voß (community) Approve
James Henstridge Approve
Review via email: mp+308017@code.launchpad.net

Commit message

Fixes http header parsing in net response.

Description of the change

Fixes http header parsing in net response.

The first index of sub_match is full match.The actual index for sub_match[key, value] starts from 1.

To post a comment you must log in.
lp:~gary-wzl77/net-cpp/fix_1631846 updated
53. By Gary.Wang

enable the unit test(get_request_against_app_store_succeeds) to verify if headers are parsed properly.

Revision history for this message
James Henstridge (jamesh) wrote :

I'm not really comfortable with the test changes. See inline comments.

review: Needs Fixing
lp:~gary-wzl77/net-cpp/fix_1631846 updated
54. By Gary.Wang

add one test case to verify header parsing properly.

55. By Gary.Wang

simplify the test case to only verify http header and avoid the noise.

Revision history for this message
James Henstridge (jamesh) wrote :

Looks good.

review: Approve
Revision history for this message
Thomas Voß (thomas-voss) wrote :

LGTM.

review: Approve
lp:~gary-wzl77/net-cpp/fix_1631846 updated
56. By Gary.Wang

add a log to check why it failed to compile against zesty.
suspects the content-length doesn't match on zesty.

57. By Gary.Wang

The value of "Content-Length" are different on xenial and zesty,
so we only check key and ignore value for it. That makes http_client_tests passed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-08-18 01:06:17 +0000
3+++ CMakeLists.txt 2016-11-08 09:19:22 +0000
4@@ -16,7 +16,7 @@
5
6 cmake_minimum_required(VERSION 3.0)
7
8-project(net-cpp VERSION 2.1.0)
9+project(net-cpp VERSION 2.2.0)
10
11 set(NET_CPP_SOVERSION 2 CACHE STRING "The version number from libnet-cpp's SONAME")
12
13
14=== modified file 'debian/changelog'
15--- debian/changelog 2016-09-13 08:55:31 +0000
16+++ debian/changelog 2016-11-08 09:19:22 +0000
17@@ -1,3 +1,11 @@
18+net-cpp (2.2.0+16.10.20160913.2-0ubuntu2) UNRELEASED; urgency=medium
19+
20+ [ Gary Wang ]
21+ * Fix http header parsing in net response.
22+ (LP: #1631846)
23+
24+ -- Gary.Wang <gary.wang@canonical.com> Mon, 10 Oct 2016 13:06:17 +0800
25+
26 net-cpp (2.1.0+16.10.20160913.2-0ubuntu1) yakkety; urgency=medium
27
28 [ Thomas Voß ]
29
30=== modified file 'src/core/net/http/impl/curl/request.h'
31--- src/core/net/http/impl/curl/request.h 2016-08-18 04:03:26 +0000
32+++ src/core/net/http/impl/curl/request.h 2016-11-08 09:19:22 +0000
33@@ -54,7 +54,7 @@
34 if (not std::regex_match(line, line + size, matches, http::header_line))
35 return std::make_tuple(std::string{}, std::string{});
36
37- return std::make_tuple(matches.str(0), matches.str(1));
38+ return std::make_tuple(matches.str(1), matches.str(2));
39 }
40
41 std::tuple<std::string, std::string, std::size_t> handle_header_line(void* data, std::size_t size, std::size_t nmemb)
42
43=== modified file 'tests/http_client_test.cpp'
44--- tests/http_client_test.cpp 2016-08-11 15:06:23 +0000
45+++ tests/http_client_test.cpp 2016-11-08 09:19:22 +0000
46@@ -551,6 +551,40 @@
47 EXPECT_EQ(url, root["url"].asString());
48 }
49
50+TEST(HttpClient, get_request_for_http_headers_checking)
51+{
52+ // We obtain a default client instance, dispatching to the default implementation.
53+ auto client = http::make_client();
54+
55+ // Url pointing to the resource we would like to access via http.
56+ auto url = std::string(httpbin::host) + httpbin::resources::get();
57+
58+ // The client mostly acts as a factory for http requests.
59+ auto request = client->head(http::Request::Configuration::from_uri_as_string(url));
60+
61+ // We finally execute the query synchronously and story the response.
62+ auto response = request->execute(default_progress_reporter);
63+
64+ // We expect the query to complete successfully
65+ EXPECT_EQ(core::net::http::Status::ok, response.status);
66+
67+ //check if the headers are parsed properly.
68+ auto headers = response.header;
69+
70+ response.header.enumerate([](const std::string& key,
71+ const std::set<std::string>& values) {
72+ for (const auto& v: values) {
73+ std::cout << "key: " << key <<" value: " << v << std::endl;
74+ }
75+ });
76+
77+ //As value of "Content-Length" are different on xenial and zesty,
78+ //so we only check key and ignore value for it.
79+ EXPECT_TRUE(headers.has("Content-Length"));
80+ EXPECT_TRUE(headers.has("Access-Control-Allow-Origin", "*"));
81+ EXPECT_TRUE(headers.has("Content-Type", core::net::http::ContentType::json));
82+}
83+
84 namespace com
85 {
86 namespace mozilla

Subscribers

People subscribed via source and target branches

to all changes: