Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Freeform object support, safe string operations, URL encoding fixes for C-libcurl #12069

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Freeform object support, safe string operations, URL encoding fixes for C-libcurl #12069

wants to merge 12 commits into from

Conversation

harlequin-tech
Copy link

@harlequin-tech harlequin-tech commented Apr 6, 2022 •
edited
Loading

  • Implements freeform object support as the json string of the object. Callers manually decode the json to extract the data. (See [BUG] [C Client] incorrect generate "object_convertToJSON" function #5735).
  • Replaces strcpy and strcat with bounded strncpy and strncat for safety.
  • Fixes URL encoding for query parameters so the keys and values can contain special characters.
  • Tested with 327 endpoint model, subset used for query parameter test and free-form objects.

@zhemant @ityuhui @michelealbano updates on behalf of https://moxion.io.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples: 
    ./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@harlequin-tech harlequin-tech changed the title Freeform object support, safe string operations, URL encoding fixes Freeform object support, safe string operations, URL encoding fixes for C-libcurl Apr 6, 2022
@ityuhui
Copy link
Contributor

ityuhui commented Apr 7, 2022

Thank you for your contribution !

Can you resove the merge confict first ?

ityuhui
ityuhui reviewed Apr 7, 2022
View reviewed changes
modules/openapi-generator/src/main/resources/C-libcurl/cJSON.c.mustache
@@ -841,12 +841,12 @@ static cJSON_bool print_string_ptr(const unsigned char * const input, printbuffe
/* empty string */
if (input == NULL)
{
output = ensure(output_buffer, sizeof("\"\""));
output = ensure(output_buffer, sizeof("\"\"")+1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The souce code comes from cJSON https://github.com/DaveGamble/cJSON. we shouldn't hack to fix it here.
Suggest fixing it in cJSON project or leave this code as is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll see if they'll accept the changes.

ityuhui
ityuhui reviewed Apr 7, 2022
View reviewed changes
modules/openapi-generator/src/main/resources/C-libcurl/api-body.mustache
@@ -325,11 +325,6 @@ end:
localVarBodyParameters,
"{{{httpMethod}}}");

{{#responses}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code has already been changed by #12056

ityuhui
ityuhui reviewed Apr 7, 2022
View reviewed changes
modules/openapi-generator/src/main/resources/C-libcurl/apiClient.c.mustache
@@ -134,6 +134,15 @@ void apiClient_free(apiClient_t *apiClient) {
free(apiClient);
}

void apiClient_free_data(apiClient_t *apiClient) {
if (apiClient->dataReceived) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where this function is being called. What's its purpse ? for free-form object parsing ?

@ityuhui
Copy link
Contributor

ityuhui commented Apr 28, 2022 •
edited
Loading

Hi @harlequin-tech

How is it going ?
If the comments are too time consuming to address, can you submit the first part of this PR (freeform object support) in a seperate PR first ? I think it's OK to merge.

@ityuhui ityuhui mentioned this pull request Jun 8, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ityuhui ityuhui ityuhui left review comments

@wing328 wing328 wing328 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@harlequin-tech @ityuhui @wing328