-
Notifications
You must be signed in to change notification settings - Fork 46
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.
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
Calling into thim agent to get collateral. #149
base: master
Are you sure you want to change the base?
Changes from 3 commits
45d7779
0e3688b
a8e56e8
d7c727c
dcc249b
81fb0bc
734a3e4
6e56c7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,7 @@ char const* curl_easy::error::what() const noexcept | |
/////////////////////////////////////////////////////////////////////////////// | ||
// curl_easy implementation | ||
/////////////////////////////////////////////////////////////////////////////// | ||
std::unique_ptr<curl_easy> curl_easy::create(const std::string& url, const std::string* const p_body) | ||
std::unique_ptr<curl_easy> curl_easy::create(const std::string& url, const std::string* const p_body, LPCWSTR httpVerb)) | ||
{ | ||
std::unique_ptr<curl_easy> easy(new curl_easy); | ||
|
||
|
@@ -92,7 +92,12 @@ std::unique_ptr<curl_easy> curl_easy::create(const std::string& url, const std:: | |
|
||
if (p_body != nullptr && !p_body->empty()) | ||
{ | ||
easy->set_opt_or_throw(CURLOPT_CUSTOMREQUEST, "GET"); | ||
if (httpVerb == L"POST") { | ||
msft-gumunjal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
easy->set_opt_or_throw(CURLOPT_POST, 1L); | ||
} | ||
else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if we have a style checker integrated into the VS solution. If we do and it passes, then ignore this comment. The convention I see in other parts of the file is to throw opening curly braces on the line after the if/else statement. We should probably keep doing that to stay consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont think we have a style checker. Let me try to add one. |
||
easy->set_opt_or_throw(CURLOPT_HTTPGET, 1L); | ||
} | ||
easy->set_opt_or_throw(CURLOPT_COPYPOSTFIELDS, p_body->c_str()); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why use
LPCWSTR
instead ofconst std::string&
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main reason for this file is to keep the structure consistent with the other curl_easy.cpp file(in src/Windows). The variable is used by "WinHttpOpenRequest" in src/Windows/curl_easy.cpp file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is Linux specific. Other parameters are std::string, so not appreciating the need for consistency with the windows version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameter is used by "WinHttpOpenRequest" and the data type is supposed to be "LPCWSTR". Either I can pass a string to the function and then convert it inside the function or just pass the desired data type. I chose the latter one because I thought that's the better out of the two. I can update if required.