-
Notifications
You must be signed in to change notification settings - Fork 1k
v3.0 upgrade libmicrohttpd to v1.0.1 #4971
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
base: v3.0
Are you sure you want to change the base?
Conversation
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.
Hello @mirostauder, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request authored by mirostauder. The primary goal of this PR is to upgrade the libmicrohttpd
dependency from version 0.9.77 to 1.0.1. This upgrade brings in the latest features and fixes from the library.
A significant change associated with this upgrade is the removal of a local patch (connection.c-snprintf-overflow.patch
) that was previously applied to fix a potential snprintf
overflow issue in the older version. The fact that this patch is removed indicates that the fix is now included natively in libmicrohttpd
version 1.0.1, simplifying our build process and relying on the upstream fix.
Additionally, there's a minor adjustment in the deps/Makefile
build steps for libmicrohttpd
, replacing a commented-out patch application with the creation of a symbolic link to the extracted library directory. This seems like a cleanup or simplification of the build setup.
Highlights
- Dependency Upgrade: Upgraded the
libmicrohttpd
dependency from version 0.9.77 to 1.0.1. - Patch Removal: Removed the local patch (
connection.c-snprintf-overflow.patch
) as the fix it provided is now included in the upstreamlibmicrohttpd
v1.0.1. - Build System Adjustment: Modified the
deps/Makefile
to replace a commented-out patch step with the creation of a symbolic link to the extractedlibmicrohttpd
directory.
Changelog
- connection.c-snprintf-overflow.patch
- Removed the local patch file that contained a fix for a potential
snprintf
overflow inconnection.c
. This fix is now part of the upstream library.
- Removed the local patch file that contained a fix for a potential
- deps/Makefile
- Replaced a commented-out patch application line with a command to create a symbolic link (
ln -fsT
) to the extractedlibmicrohttpd
directory.
- Replaced a commented-out patch application line with a command to create a symbolic link (
- deps/libmicrohttpd/libmicrohttpd
- Updated the version string reference from
libmicrohttpd-0.9.77
tolibmicrohttpd-1.0.1/
.
- Updated the version string reference from
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Old library stays,
New version comes with fixes,
Patch file removed.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request upgrades libmicrohttpd
from v0.9.77 to v1.0.1. The changes include updating the Makefile to handle the new version and removing a patch file previously applied to libmicrohttpd
.
The use of a symbolic link in deps/Makefile
for the libmicrohttpd
directory is a good improvement for maintainability, making the build process cleaner and more robust to version changes.
It's noted that the pull request description is currently empty. For future changes, especially dependency upgrades, it would be very helpful to include:
- The rationale for the upgrade (e.g., security fixes, bug fixes, new features needed).
- Confirmation that any removed patches are indeed obsolete due to fixes in the new version of the dependency.
- Any potential impacts or testing done.
My main review point focuses on ensuring the removed patch is no longer necessary.
Summary of Findings
- Patch Obsolescence Confirmation: The
connection.c-snprintf-overflow.patch
is no longer applied as part of thelibmicrohttpd
v1.0.1 upgrade. It is critical to confirm that the underlying issue (likely asnprintf
buffer overflow) addressed by this patch is definitively fixed in the upstream v1.0.1 release to prevent any security regressions. A comment has been added todeps/Makefile
regarding this. - Makefile Improvement for libmicrohttpd: The change in
deps/Makefile
to useln -fsT libmicrohttpd-*/ libmicrohttpd
creates a stable symlinklibmicrohttpd
pointing to the version-specific extracted directory. This is a good practice that improves the maintainability and robustness of the build script when updating library versions. - Removal of Patch File: The file
deps/libmicrohttpd/connection.c-snprintf-overflow.patch
has been deleted. This is consistent with no longer applying the patch in the Makefile. The validity of this removal depends on the issue being fixed inlibmicrohttpd
v1.0.1. - Version File Content Change: The content of the file
deps/libmicrohttpd/libmicrohttpd
was updated fromlibmicrohttpd-0.9.77
tolibmicrohttpd-1.0.1/
. The addition of a trailing slash is noted. If this file is parsed by any scripts, ensure they can handle this new format. This was considered low severity and not commented on directly due to review settings.
Merge Readiness
This pull request primarily focuses on upgrading the libmicrohttpd
dependency to version 1.0.1. The Makefile changes to support this are generally positive, particularly the use of a symbolic link for the library directory.
However, a key concern is the removal of the connection.c-snprintf-overflow.patch
. Before this PR can be safely merged, it is essential to confirm that the issue addressed by this patch is indeed resolved in libmicrohttpd
v1.0.1. Without this confirmation, there's a risk of reintroducing a vulnerability.
Therefore, I recommend that this confirmation be provided and verified before merging. As a reviewer, I am not authorized to approve pull requests, but I advise that this point be thoroughly addressed to maintain code quality and security.
@@ -133,7 +133,7 @@ curl: curl/curl/lib/.libs/libcurl.a | |||
libmicrohttpd/libmicrohttpd/src/microhttpd/.libs/libmicrohttpd.a: | |||
cd libmicrohttpd && rm -rf libmicrohttpd-*/ || true | |||
cd libmicrohttpd && tar -zxf libmicrohttpd-*.tar.gz | |||
# cd libmicrohttpd/libmicrohttpd && patch src/microhttpd/connection.c < ../connection.c-snprintf-overflow.patch | |||
cd libmicrohttpd && ln -fsT libmicrohttpd-*/ libmicrohttpd |
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 upgrade to libmicrohttpd
v1.0.1 involves removing the application of connection.c-snprintf-overflow.patch
(previously on the old line 136) and introducing a symbolic link here for the library directory.
While the symlink approach (ln -fsT libmicrohttpd-*/ libmicrohttpd
) is a good enhancement for managing the library version, it's crucial to ensure the removed patch is genuinely obsolete.
Could you please confirm that the snprintf
overflow issue, which connection.c-snprintf-overflow.patch
was intended to fix, has been addressed in libmicrohttpd
v1.0.1? Verifying this is essential to prevent the reintroduction of any potential vulnerabilities (like buffer overflows) that the patch might have mitigated.
No description provided.