- Notifications
You must be signed in to change notification settings - Fork2.5k
http: Initialize ‘on_status’ when using the http-parser backend.#6870
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
http: Initialize ‘on_status’ when using the http-parser backend.#6870
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Fixes a bug likely introduced ind396819 (in 1.8.1) whereby‘proxy_settings.on_status’ would be left uninitialized when using the‘http-parser’ backend, eventually leading to a segfault in‘http_parser_execute’. Valgrind would report use of the uninitializedvalue like so: Conditional jump or move depends on uninitialised value(s) at 0x50CD533: http_parser_execute (http_parser.c:910) by 0x4928504: git_http_parser_execute (httpparser.c:82) by 0x4925C42: client_read_and_parse (httpclient.c:1178) by 0x4926F27: git_http_client_read_response (httpclient.c:1458) by 0x49255FE: http_stream_read (http.c:427) by 0x4929B90: git_smart__recv (smart.c:29) by 0x492C147: git_smart__store_refs (smart_protocol.c:58) by 0x4929F6C: git_smart__connect (smart.c:171) by 0x4904DCE: git_remote_connect_ext (remote.c:963) by 0x48A15D2: clone_into (clone.c:449) by 0x48A15D2: git__clone (clone.c:546) by 0x4010E9: main (libgit2-proxy.c:20)
bbd0d7d
toea7e18e
CompareThere 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.
Instead of trying to set individual members, maybe we should just memset the whole things to zero (which emulates our prior behavior). I suggest this because the http-parser that we used to use predated the chunk boundaries processing that came from the proxygen fork...? So not all http-parser implementations (including the one that we previously shipped) seem to have this chunk completion callback.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thanks for catching this and reporting it. 🙏 |
Co-authored-by: Edward Thomson <ethomson@edwardthomson.com>
Sure, using |
Thanks for this! |
403a03b
intolibgit2:mainUh oh!
There was an error while loading.Please reload this page.
Fixes a bug likely introduced in
d396819 (in1.8.1) whereby
proxy_settings.on_status
would be left uninitialized when using the ‘http-parser’ backend (-DUSE_HTTP_PARSER=http-parser
), eventually leading to a segfault inhttp_parser_execute
. Valgrind would report use of the uninitialized value like so:A simple reproducer is this program:
Note that this bug does not exist when building libgit2 with
-DUSE_HTTP_PARSER=llhttp
.