refactor(core): replace lib curl calls with urlHelper#218
refactor(core): replace lib curl calls with urlHelper#218Lakshmi97-velampati wants to merge 4 commits into
Conversation
In CertifierOperationalCredentialsIssuer replaced the lib curl calls with the urlHelper functions to get the benefits of the DEBUGFUNCTION opt. Refs: BARTON-330
Lakshmi97-velampati
requested review from
cleithner-comcast,
kfundecmcsa,
mkkoch,
rchowdcmcsa and
tleacmcsa
as code owners
There was a problem hiding this comment.
Pull request overview
This PR refactors CertifierOperationalCredentialsIssuer to stop using direct libcurl calls and instead perform the certifier HTTP request through urlHelper, aligning the Matter operational credentials flow with the project’s shared HTTP utility (and enabling the DEBUGFUNCTION benefits noted in the PR description).
Changes:
- Removed libcurl dependency/typedefs from the issuer header.
- Reworked the NOC fetch path to build an
icLinkedListof HTTP headers and execute the POST viaurlHelperExecuteRequestHeaders. - Updated response handling to use the returned body buffer and the HTTP status code from
urlHelper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/subsystems/matter/delegates/default/CertifierOperationalCredentialsIssuer.hpp | Removes libcurl include and CurlEasy alias to decouple the interface from libcurl. |
| core/src/subsystems/matter/delegates/default/CertifierOperationalCredentialsIssuer.cpp | Replaces the libcurl-based HTTP POST with urlHelperExecuteRequestHeaders, including header construction and response handling. |
Lakshmi97-velampati
changed the title
tleacmcsa
left a comment
tleacmcsa
left a comment
There was a problem hiding this comment.
Looks good to me. How was it tested? You out C++'d me in some spots :-D
Just ran integration tests which will cover the commissioning flow. |
| long httpCode; | ||
| curl_easy_getinfo(curl.get(), CURLINFO_RESPONSE_CODE, &httpCode); | ||
|
|
||
| if (httpCode >= 400) |
There was a problem hiding this comment.
Minor thought: we might be able to get a bit more value out of httpCode (e.g. distinguishing 4xx vs 5xx) to better understand whether failures are coming from our request vs the server.
It does not in this case. This module is for "production" or "real device" use. For development and the test suite, we use SelfSignedCertifierOperationalCredentialsIssuer to avoid reaching out to a pki server in tests. To test this properly, you need to commission a real matter device (or a locally findable matter sample app would be fine) on an activated installation. Because this default module particularly hits the comcast xpki server, and I believe that server demands an xbo correlated mac and account id, I think your easiest path of testing would be with a real gateway. |
…ntialsIssuer Log client errors (4xx) and server errors (5xx) separately to make it easier to determine whether NOC request failures originate from a bad request on our side or a problem on the certifier server.
Lakshmi97-velampati
dismissed stale reviews from tleacmcsa and kfundecmcsa
via
2022c4f
| private: | ||
| using CurlEasy = std::unique_ptr<CURL, decltype(&curl_easy_cleanup)>; | ||
| using Pkcs7 = std::unique_ptr<PKCS7, decltype(&PKCS7_free)>; | ||
|
|
Sure will test and update. |
In CertifierOperationalCredentialsIssuer replaced the lib curl calls with the urlHelper functions to get the benefits of the DEBUGFUNCTION opt.
Refs: BARTON-330