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.

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

Server: version bump for httplib and json #6169

Merged
merged 4 commits into from
Mar 20, 2024

Conversation

ngxson
Copy link
Collaborator

@ngxson ngxson commented Mar 19, 2024

  • Update httplib to 0.15.3
  • Update json to 3.11.3

Closes #6162

@ngxson ngxson requested review from slaren and phymbert March 19, 2024 20:35
@ngxson ngxson changed the title Berver: version bump for httplib and json Server: version bump for httplib and json Mar 19, 2024
@slaren
Copy link
Member

slaren commented Mar 19, 2024

Build is failing due to an unused parameter in a 3rd party library. Coincidentally, after finding 188 instances of GGML_UNUSED in ggml-cuda.cu, I was about to suggest to remove this warning entirely, because IMO it causes more harm than good to the code quality.

 examples/server/httplib.h:4797:45: error: unused parameter ‘content_length’ [-Werror=unused-parameter]

@slaren slaren requested a review from ggerganov March 19, 2024 20:44
@ngxson
Copy link
Collaborator Author

ngxson commented Mar 19, 2024

Yeah I got the same error on local build, seems like because the parameter is only being used by assert(), which is not present in our build. At first, I didn't want to modify the code because I didn't want to touch an external code. But if that breaks the CI, we have no choice then.

@phymbert
Copy link
Collaborator

Note the windows server tests is failing for something not related. I will fix it later on

@phymbert
Copy link
Collaborator

Note the windows server tests is failing for something not related. I will fix it later on

@ngxson Tests have been:

Please merge master into this to be sure all is OK on server tests, then I am happy to merge this important upstream security upgrade

@ggerganov
Copy link
Member

The change in e5ed307 does not work in Debug builds. Use (void)(content_length);

We can alternatively do as proposed by @slaren - disable the warning and eventually get rid of the unused calls

@ngxson
Copy link
Collaborator Author

ngxson commented Mar 20, 2024

I added (void)(content_length); because it seems to be the most non-destructive choice for now.

I also merged latest master to pick up the changes that @phymbert mentioned, hopefully the CI will pass

@ngxson ngxson merged commit 91f8ad1 into ggml-org:master Mar 20, 2024
54 checks passed
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
* server: version bump for httplib and json

* fix build

* bring back content_length
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 3, 2024
* server: version bump for httplib and json

* fix build

* bring back content_length
tybalex pushed a commit to rubra-ai/tools.cpp that referenced this pull request Apr 17, 2024
* server: version bump for httplib and json

* fix build

* bring back content_length
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server may have a directory traversal vulnerability fixed upstream
4 participants