Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Conversation

civodul
Copy link
Contributor

Fixes a bug likely introduced in
d396819 (in1.8.1) wherebyproxy_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:

   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)

A simple reproducer is this program:

#include<git2.h>#include<assert.h>#include<stdio.h>intmain (){interr;err=git_libgit2_init ();assert (err==1);git_clone_optionsopts;err=git_clone_init_options (&opts,GIT_CLONE_OPTIONS_VERSION);assert (err==0);opts.fetch_opts.proxy_opts.type=GIT_PROXY_SPECIFIED;opts.fetch_opts.proxy_opts.url="http://example.org/";git_repository*repo;err=git_clone (&repo,"http://example.org/whatever.git","/tmp/example",&opts);assert (err!=0);constgit_error*gerr;gerr=giterr_last ();fprintf (stderr,"last error: %s\n",gerr->message);return0;}

Note that this bug does not exist when building libgit2 with-DUSE_HTTP_PARSER=llhttp.

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)
@civodulcivodulforce-pushed thefix-uninitialized-http-parser-proxy-settings branch frombbd0d7d toea7e18eCompareAugust 29, 2024 11:51
Copy link
Member

@ethomsonethomson left a 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.

@ethomson
Copy link
Member

Thanks for catching this and reporting it. 🙏

Co-authored-by: Edward Thomson <ethomson@edwardthomson.com>
@civodul
Copy link
ContributorAuthor

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.

Sure, usingmemset makes sense to me (I wasn't aware of the multiple http-parser implementations and thought setting individual fields was a stylistic choice).

@ethomson
Copy link
Member

Thanks for this!

@ethomsonethomson merged commit403a03b intolibgit2:mainSep 3, 2024
19 checks passed
@ethomsonethomson added the bug labelSep 29, 2024
@civodulcivodul deleted the fix-uninitialized-http-parser-proxy-settings branchMay 27, 2025 15:23
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ethomsonethomsonethomson left review comments

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

Successfully merging this pull request may close these issues.

2 participants
@civodul@ethomson

[8]ページ先頭

©2009-2025 Movatter.jp