-
Notifications
You must be signed in to change notification settings - Fork 1
test-cov1: Fixing medium and high issue[DONOT MERGE] #108
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
Changes from all commits
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -330,7 +330,6 @@ void NativeJSRenderer::createApplicationInternal(ApplicationRequest& appRequest) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| context->setCreateApplicationEndTime(endTime, id); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mContextMap[id].context=context; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mUserMutex.unlock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -445,7 +444,7 @@ void NativeJSRenderer::terminateApplicationInternal(ApplicationRequest& AppReque | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url.c_str()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url.c_str()); | |
| NativeJSLogger::log(ERROR, "Unable to find application with id: %d\n", id); |
Copilot
AI
Jan 19, 2026
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.
In NativeJSRenderer::run(), the id used to construct ApplicationRequest appRequest(id, RUN, mTestFileName, ...) is never initialized, so mId and all subsequent uses of this id (context map access, logging, etc.) rely on an indeterminate value. This is undefined behavior and can result in collisions or hard-to-reproduce failures; the id should be obtained from a defined source (e.g., a counter or createApplication-like path) before being passed into ApplicationRequest.
Copilot
AI
Jan 19, 2026
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.
In NativeJSRenderer::downloadFile, when any curl_easy_setopt call fails you log an error and call curl_easy_cleanup(curl) but then continue to use curl for subsequent curl_easy_setopt calls and later curl_easy_perform, which is undefined behavior because the handle has been cleaned up. After a failed curl_easy_setopt, the function should stop using the handle (e.g., return false immediately or guard the curl_easy_perform path with a success flag) instead of continuing with a freed/invalid CURL*.
| } | |
| res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYHOST: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYPEER: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0"); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_USERAGENT: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_PROXY, ""); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_PROXY: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| return ret; | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYHOST: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| return ret; | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYPEER: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| return ret; | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0"); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_USERAGENT: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| return ret; | |
| } | |
| res = curl_easy_setopt(curl, CURLOPT_PROXY, ""); | |
| if (res != CURLE_OK) { | |
| NativeJSLogger::log(ERROR, "Failed to set CURLOPT_PROXY: %s\n", curl_easy_strerror(res)); | |
| curl_easy_cleanup(curl); | |
| return ret; |
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 error message in
JsonWrap::getUint32prints"Error: " << name << "is not a Uint32_t", which is missing a space beforeisand uses a non-standard type spelling (Uint32_t). For clarity and consistency with the actual type, consider changing the message to something like"Error: <name> is not a uint32_t".