Feature/auth builder #31
Conversation
src/api.cpp
Outdated
|
|
||
| std::stringstream queryStrStream; | ||
| for (const auto& pair : queryParams) { | ||
| queryStrStream << pair.first << "=" << pair.second << "&"; |
There was a problem hiding this comment.
Potential future footgun with unescaped query parameters
Also this seems like a candidate to be replaced with std::string queryString()
There was a problem hiding this comment.
Updated this to make a call to queryString (which now does url encoding of the pair values)
| return url; | ||
| } | ||
|
|
||
| std::string MarketplaceApi::getGatewayUrl( |
There was a problem hiding this comment.
Is there future logic that is planned which will make this function materially different from getAuthUrl?
There was a problem hiding this comment.
Potentially yes, as the calls to the gateway API have various standard parameters such as limits/offsets. My thought is to standardize this in the future for easier/more clear calls to the various API endpoints versus the calls done to the OAuth process
src/util.cpp
Outdated
| { | ||
| std::stringstream queryStrStream; | ||
| for (const auto& pair : params) { | ||
| queryStrStream << "\"" << pair.first << "\": \"" << pair.second << "\","; |
There was a problem hiding this comment.
Unescaped JSON keys/values too
There was a problem hiding this comment.
Fixed by doing a round trip with nlohmann::json from std::map --> json object --> std::string which escapes json values.
| std::map<std::string, std::string> const& queryParams | ||
| ) | ||
| { | ||
| std::string endpoint = segments.size() > 0 ? std::accumulate(std::next(segments.begin()), segments.end(), segments[0], |
There was a problem hiding this comment.
I know this is technically not "bad" code, but I prefer when algorithms like this are spun out into a well named helper function; especially when it's a pattern used in multiple places for a relatively well understood high level operation like this.
There was a problem hiding this comment.
I've noted this, and will revisit.
… escaping for all json post bodies.
No description provided.