Skip to content

Commit 5b67e39

Browse files
committed
Merge remote-tracking branch 'upstream/main' into clp_s_sfa
2 parents 3d51d22 + 9f6a6ce commit 5b67e39

File tree

10 files changed

+166
-22
lines changed

10 files changed

+166
-22
lines changed

.github/workflows/clp-core-build-macos.yaml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ on:
88
- "components/core/CMakeLists.txt"
99
- "components/core/src/**"
1010
- "components/core/tests/**"
11-
- "components/core/tools/scripts/lib_install/macos-12/**"
11+
- "components/core/tools/scripts/lib_install/macos/**"
1212
- "components/core/tools/scripts/deps-download/**"
1313
- "components/core/tools/scripts/utils/build-and-run-unit-tests.py"
1414
- "deps-tasks.yml"
@@ -21,7 +21,7 @@ on:
2121
- "components/core/CMakeLists.txt"
2222
- "components/core/src/**"
2323
- "components/core/tests/**"
24-
- "components/core/tools/scripts/lib_install/macos-12/**"
24+
- "components/core/tools/scripts/lib_install/macos/**"
2525
- "components/core/tools/scripts/deps-download/**"
2626
- "components/core/tools/scripts/utils/build-and-run-unit-tests.py"
2727
- "deps-tasks.yml"
@@ -38,8 +38,9 @@ jobs:
3838
build-macos:
3939
strategy:
4040
matrix:
41+
runner: ["macos-13", "macos-14"]
4142
use_shared_libs: [true, false]
42-
runs-on: "macos-12"
43+
runs-on: "${{matrix.runner}}"
4344
steps:
4445
- uses: "actions/checkout@v4"
4546
with:
@@ -55,7 +56,7 @@ jobs:
5556
rm -f /usr/local/bin/python3*
5657
5758
- name: "Install dependencies"
58-
run: "./components/core/tools/scripts/lib_install/macos-12/install-all.sh"
59+
run: "./components/core/tools/scripts/lib_install/macos/install-all.sh"
5960

6061
- run: "./tools/scripts/deps-download/init.sh"
6162
shell: "bash"

.github/workflows/clp-core-build.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ on:
1111
- "deps-tasks.yml"
1212
- "Taskfile.yml"
1313
- "tools/scripts/deps-download/**"
14-
- "!components/core/tools/scripts/lib_install/macos-12/**"
14+
- "!components/core/tools/scripts/lib_install/macos/**"
1515
push:
1616
paths:
1717
- ".github/actions/clp-core-build/action.yaml"
@@ -22,7 +22,7 @@ on:
2222
- "deps-tasks.yml"
2323
- "Taskfile.yml"
2424
- "tools/scripts/deps-download/**"
25-
- "!components/core/tools/scripts/lib_install/macos-12/**"
25+
- "!components/core/tools/scripts/lib_install/macos/**"
2626
workflow_dispatch:
2727

2828
env:

components/core/src/clp/CurlDownloadHandler.cpp

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
#include "CurlDownloadHandler.hpp"
22

3+
#include <algorithm>
4+
#include <array>
5+
#include <cctype>
36
#include <chrono>
47
#include <cstddef>
58
#include <memory>
9+
#include <optional>
610
#include <string>
711
#include <string_view>
12+
#include <unordered_map>
13+
#include <unordered_set>
814
#include <utility>
915

1016
#include <curl/curl.h>
17+
#include <fmt/core.h>
18+
19+
#include "ErrorCode.hpp"
1120

1221
namespace clp {
1322
CurlDownloadHandler::CurlDownloadHandler(
@@ -19,7 +28,8 @@ CurlDownloadHandler::CurlDownloadHandler(
1928
size_t offset,
2029
bool disable_caching,
2130
std::chrono::seconds connection_timeout,
22-
std::chrono::seconds overall_timeout
31+
std::chrono::seconds overall_timeout,
32+
std::optional<std::unordered_map<std::string, std::string>> const& http_header_kv_pairs
2333
)
2434
: m_error_msg_buf{std::move(error_msg_buf)} {
2535
if (nullptr != m_error_msg_buf) {
@@ -48,13 +58,55 @@ CurlDownloadHandler::CurlDownloadHandler(
4858
m_easy_handle.set_option(CURLOPT_TIMEOUT, static_cast<long>(overall_timeout.count()));
4959

5060
// Set up http headers
61+
constexpr std::string_view cRangeHeaderName{"range"};
62+
constexpr std::string_view cCacheControlHeaderName{"cache-control"};
63+
constexpr std::string_view cPragmaHeaderName{"pragma"};
64+
std::unordered_set<std::string_view> const reserved_headers{
65+
cRangeHeaderName,
66+
cCacheControlHeaderName,
67+
cPragmaHeaderName
68+
};
5169
if (0 != offset) {
52-
std::string const range{"Range: bytes=" + std::to_string(offset) + "-"};
53-
m_http_headers.append(range);
70+
m_http_headers.append(fmt::format("{}: bytes={}-", cRangeHeaderName, offset));
5471
}
5572
if (disable_caching) {
56-
m_http_headers.append("Cache-Control: no-cache");
57-
m_http_headers.append("Pragma: no-cache");
73+
m_http_headers.append(fmt::format("{}: no-cache", cCacheControlHeaderName));
74+
m_http_headers.append(fmt::format("{}: no-cache", cPragmaHeaderName));
75+
}
76+
if (http_header_kv_pairs.has_value()) {
77+
for (auto const& [key, value] : http_header_kv_pairs.value()) {
78+
// HTTP header field-name (key) is case-insensitive:
79+
// https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
80+
// Therefore, we convert keys to lowercase for comparison with the reserved keys.
81+
// NOTE: We do not check for duplicate keys due to case insensitivity, leaving duplicate
82+
// handling to the server.
83+
auto lower_key{key};
84+
std::transform(
85+
lower_key.begin(),
86+
lower_key.end(),
87+
lower_key.begin(),
88+
[](unsigned char c) -> char {
89+
// Implicitly cast the input character into `unsigned char` to avoid UB:
90+
// https://en.cppreference.com/w/cpp/string/byte/tolower
91+
return static_cast<char>(std::tolower(c));
92+
}
93+
);
94+
if (reserved_headers.contains(lower_key) || value.ends_with("\r\n")) {
95+
throw CurlOperationFailed(
96+
ErrorCode_Failure,
97+
__FILE__,
98+
__LINE__,
99+
CURLE_BAD_FUNCTION_ARGUMENT,
100+
fmt::format(
101+
"`CurlDownloadHandler` failed to construct with the following "
102+
"invalid header: {}:{}",
103+
key,
104+
value
105+
)
106+
);
107+
}
108+
m_http_headers.append(fmt::format("{}: {}", key, value));
109+
}
58110
}
59111
if (false == m_http_headers.is_empty()) {
60112
m_easy_handle.set_option(CURLOPT_HTTPHEADER, m_http_headers.get_raw_list());

components/core/src/clp/CurlDownloadHandler.hpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
#include <chrono>
66
#include <cstddef>
77
#include <memory>
8+
#include <optional>
9+
#include <string>
810
#include <string_view>
11+
#include <unordered_map>
912

1013
#include <curl/curl.h>
1114

@@ -53,6 +56,9 @@ class CurlDownloadHandler {
5356
* Doc: https://curl.se/libcurl/c/CURLOPT_CONNECTTIMEOUT.html
5457
* @param overall_timeout Maximum time that the transfer may take. Note that this includes
5558
* `connection_timeout`. Doc: https://curl.se/libcurl/c/CURLOPT_TIMEOUT.html
59+
* @param http_header_kv_pairs Key-value pairs representing HTTP headers to pass to the server
60+
* in the download request. Doc: https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html
61+
* @throw CurlOperationFailed if an error occurs.
5662
*/
5763
explicit CurlDownloadHandler(
5864
std::shared_ptr<ErrorMsgBuf> error_msg_buf,
@@ -63,7 +69,9 @@ class CurlDownloadHandler {
6369
size_t offset = 0,
6470
bool disable_caching = false,
6571
std::chrono::seconds connection_timeout = cDefaultConnectionTimeout,
66-
std::chrono::seconds overall_timeout = cDefaultOverallTimeout
72+
std::chrono::seconds overall_timeout = cDefaultOverallTimeout,
73+
std::optional<std::unordered_map<std::string, std::string>> const& http_header_kv_pairs
74+
= std::nullopt
6775
);
6876

6977
// Disable copy/move constructors/assignment operators

components/core/src/clp/NetworkReader.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
#include <memory>
77
#include <mutex>
88
#include <optional>
9+
#include <string>
910
#include <string_view>
11+
#include <unordered_map>
12+
#include <utility>
1013

1114
#include <curl/curl.h>
1215

@@ -118,7 +121,8 @@ NetworkReader::NetworkReader(
118121
std::chrono::seconds overall_timeout,
119122
std::chrono::seconds connection_timeout,
120123
size_t buffer_pool_size,
121-
size_t buffer_size
124+
size_t buffer_size,
125+
std::optional<std::unordered_map<std::string, std::string>> http_header_kv_pairs
122126
)
123127
: m_src_url{src_url},
124128
m_offset{offset},
@@ -130,7 +134,12 @@ NetworkReader::NetworkReader(
130134
for (size_t i = 0; i < m_buffer_pool_size; ++i) {
131135
m_buffer_pool.emplace_back(m_buffer_size);
132136
}
133-
m_downloader_thread = std::make_unique<DownloaderThread>(*this, offset, disable_caching);
137+
m_downloader_thread = std::make_unique<DownloaderThread>(
138+
*this,
139+
offset,
140+
disable_caching,
141+
std::move(http_header_kv_pairs)
142+
);
134143
m_downloader_thread->start();
135144
}
136145

@@ -215,7 +224,8 @@ auto NetworkReader::DownloaderThread::thread_method() -> void {
215224
m_offset,
216225
m_disable_caching,
217226
m_reader.m_connection_timeout,
218-
m_reader.m_overall_timeout
227+
m_reader.m_overall_timeout,
228+
m_http_header_kv_pairs
219229
};
220230
auto const ret_code{curl_handler.perform()};
221231
// Enqueue the last filled buffer, if any

components/core/src/clp/NetworkReader.hpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include <span>
1414
#include <string>
1515
#include <string_view>
16+
#include <unordered_map>
17+
#include <utility>
1618
#include <vector>
1719

1820
#include <curl/curl.h>
@@ -94,6 +96,8 @@ class NetworkReader : public ReaderInterface {
9496
* Doc: https://curl.se/libcurl/c/CURLOPT_CONNECTTIMEOUT.html
9597
* @param buffer_pool_size The required number of buffers in the buffer pool.
9698
* @param buffer_size The size of each buffer in the buffer pool.
99+
* @param http_header_kv_pairs Key-value pairs representing HTTP headers to pass to the server
100+
* in the download request. Doc: https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html
97101
*/
98102
explicit NetworkReader(
99103
std::string_view src_url,
@@ -103,7 +107,9 @@ class NetworkReader : public ReaderInterface {
103107
std::chrono::seconds connection_timeout
104108
= CurlDownloadHandler::cDefaultConnectionTimeout,
105109
size_t buffer_pool_size = cDefaultBufferPoolSize,
106-
size_t buffer_size = cDefaultBufferSize
110+
size_t buffer_size = cDefaultBufferSize,
111+
std::optional<std::unordered_map<std::string, std::string>> http_header_kv_pairs
112+
= std::nullopt
107113
);
108114

109115
// Destructor
@@ -242,11 +248,19 @@ class NetworkReader : public ReaderInterface {
242248
* @param reader
243249
* @param offset Index of the byte at which to start the download.
244250
* @param disable_caching Whether to disable caching.
251+
* @param http_header_kv_pairs Key-value pairs representing HTTP headers to pass to the
252+
* server in the download request. Doc: https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html
245253
*/
246-
DownloaderThread(NetworkReader& reader, size_t offset, bool disable_caching)
254+
DownloaderThread(
255+
NetworkReader& reader,
256+
size_t offset,
257+
bool disable_caching,
258+
std::optional<std::unordered_map<std::string, std::string>> http_header_kv_pairs
259+
)
247260
: m_reader{reader},
248261
m_offset{offset},
249-
m_disable_caching{disable_caching} {}
262+
m_disable_caching{disable_caching},
263+
m_http_header_kv_pairs{std::move(http_header_kv_pairs)} {}
250264

251265
private:
252266
// Methods implementing `clp::Thread`
@@ -255,6 +269,7 @@ class NetworkReader : public ReaderInterface {
255269
NetworkReader& m_reader;
256270
size_t m_offset{0};
257271
bool m_disable_caching{false};
272+
std::optional<std::unordered_map<std::string, std::string>> m_http_header_kv_pairs;
258273
};
259274

260275
/**

components/core/tests/test-NetworkReader.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@
66
#include <string>
77
#include <string_view>
88
#include <thread>
9+
#include <unordered_map>
910
#include <vector>
1011

1112
#include <Catch2/single_include/catch2/catch.hpp>
1213
#include <curl/curl.h>
14+
#include <fmt/core.h>
15+
#include <json/single_include/nlohmann/json.hpp>
1316

1417
#include "../src/clp/Array.hpp"
1518
#include "../src/clp/CurlDownloadHandler.hpp"
@@ -188,3 +191,58 @@ TEST_CASE("network_reader_illegal_offset", "[NetworkReader]") {
188191
size_t pos{};
189192
REQUIRE((clp::ErrorCode_Failure == reader.try_get_pos(pos)));
190193
}
194+
195+
TEST_CASE("network_reader_with_valid_http_header_kv_pairs", "[NetworkReader]") {
196+
std::unordered_map<std::string, std::string> valid_http_header_kv_pairs;
197+
// We use httpbin (https://httpbin.org/) to test the user-specified headers. On success, it is
198+
// supposed to respond all the user-specified headers as key-value pairs in JSON form.
199+
constexpr int cNumHttpHeaderKeyValuePairs{10};
200+
for (size_t i{0}; i < cNumHttpHeaderKeyValuePairs; ++i) {
201+
valid_http_header_kv_pairs.emplace(
202+
fmt::format("Unit-Test-Key{}", i),
203+
fmt::format("Unit-Test-Value{}", i)
204+
);
205+
}
206+
clp::NetworkReader reader{
207+
"https://httpbin.org/headers",
208+
0,
209+
false,
210+
clp::CurlDownloadHandler::cDefaultOverallTimeout,
211+
clp::CurlDownloadHandler::cDefaultConnectionTimeout,
212+
clp::NetworkReader::cDefaultBufferPoolSize,
213+
clp::NetworkReader::cDefaultBufferSize,
214+
valid_http_header_kv_pairs
215+
};
216+
auto const content = nlohmann::json::parse(get_content(reader));
217+
auto const& headers{content.at("headers")};
218+
REQUIRE(assert_curl_error_code(CURLE_OK, reader));
219+
for (auto const& [key, value] : valid_http_header_kv_pairs) {
220+
REQUIRE((value == headers.at(key).get<std::string_view>()));
221+
}
222+
}
223+
224+
TEST_CASE("network_reader_with_illegal_http_header_kv_pairs", "[NetworkReader]") {
225+
auto illegal_header_kv_pairs = GENERATE(
226+
// The following headers are determined by offset and disable_cache, which should not be
227+
// overridden by user-defined headers.
228+
std::unordered_map<std::string, std::string>{{"Range", "bytes=100-"}},
229+
std::unordered_map<std::string, std::string>{{"RAnGe", "bytes=100-"}},
230+
std::unordered_map<std::string, std::string>{{"Cache-Control", "no-cache"}},
231+
std::unordered_map<std::string, std::string>{{"Pragma", "no-cache"}},
232+
// The CRLF-terminated headers should be rejected.
233+
std::unordered_map<std::string, std::string>{{"Legal-Name", "CRLF\r\n"}}
234+
);
235+
clp::NetworkReader reader{
236+
"https://httpbin.org/headers",
237+
0,
238+
false,
239+
clp::CurlDownloadHandler::cDefaultOverallTimeout,
240+
clp::CurlDownloadHandler::cDefaultConnectionTimeout,
241+
clp::NetworkReader::cDefaultBufferPoolSize,
242+
clp::NetworkReader::cDefaultBufferSize,
243+
illegal_header_kv_pairs
244+
};
245+
auto const content = get_content(reader);
246+
REQUIRE(content.empty());
247+
REQUIRE(assert_curl_error_code(CURLE_BAD_FUNCTION_ARGUMENT, reader));
248+
}
File renamed without changes.

docs/src/dev-guide/components-core/index.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ A handful of packages and libraries are required to build CLP. There are two opt
5454
See the relevant README for your OS:
5555

5656
* [CentOS Stream 9](centos-stream-9-deps-install)
57-
* [macOS 12](macos12-deps-install)
57+
* [macOS](macos-deps-install)
5858
* [Ubuntu 20.04](ubuntu-focal-deps-install)
5959
* [Ubuntu 22.04](ubuntu-jammy-deps-install)
6060

@@ -98,7 +98,7 @@ the relevant paths on your machine.
9898
:hidden:
9999

100100
centos-stream-9-deps-install
101-
macos12-deps-install
101+
macos-deps-install
102102
ubuntu-focal-deps-install
103103
ubuntu-jammy-deps-install
104104
regex-utils

docs/src/dev-guide/components-core/macos12-deps-install.md renamed to docs/src/dev-guide/components-core/macos-deps-install.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# macOS 12 setup
1+
# macOS setup
22

33
To install the dependencies required to build clp-core, follow the steps below.
44
These same steps are used by our [GitHub workflow][gh-workflow].
@@ -13,7 +13,7 @@ will not install any dependencies you don't expect.
1313
To install all dependencies, run:
1414

1515
```shell
16-
components/core/tools/scripts/lib_install/macos-12/install-all.sh
16+
components/core/tools/scripts/lib_install/macos/install-all.sh
1717
```
1818

1919
[gh-workflow]: https://github.com/y-scope/clp/blob/main/.github/workflows/clp-core-build-macos.yaml

0 commit comments

Comments
 (0)