On Wed, Dec 3, 2014 at 2:00 PM, Michael Blume <blume.mike@xxxxxxxxx> wrote: > On Wed, Dec 3, 2014 at 1:37 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: >> >>>> @@ -515,6 +517,9 @@ void http_cleanup(void) >>>> cert_auth.password = NULL; >>>> } >>>> ssl_cert_password_required = 0; >>>> + >>>> + if (cached_accept_language) >>>> + strbuf_release(cached_accept_language); >>> >>> Junio already mentioned that this is leaking the memory of the strbuf >>> struct itself which was xmalloc()'d by get_accept_language(). >> >> I actually didn't ;-) A singleton cached_accept_language strbuf >> itself being kept around, with its reuse by get_accept_language(), >> is fine and is not a leak. But clearing the strbuf alone will >> introduce correctness problem---the second HTTP connection will see >> an empty strbuf, get_accept_language() will say "we've already >> computed and the header we must issue is an empty string", which is >> not correct. >> >> In the fix-up "SQUASH???" commit I queued on top of this patch on >> 'pu', I had to run "sort -u" on the output to the standard error >> stream, as there seemed to be two HTTP connections and the actual >> output had two headers, even though the test expected only one in >> the output. I suspect that it is a fallout from this bug that the >> original code passed that test that expects only one. >> >>>> +static struct strbuf *get_accept_language(void) >>> >>> I find this API a bit strange. Use of strbuf to construct the returned >>> string is an implementation detail of this function. From the caller's >>> point of view, it should just be receiving a constant string: one >>> which it needs neither to modify nor free. Also, if the caller were to >>> modify the returned strbuf for some reason, then that modification >>> would impact all future calls to get_accept_language() since the >>> strbuf is 'static' and not recomputed. Instead, I would expect the >>> declaration to be: >>> >>> static const char *get_accept_language(void) >> >> Makes sense to me. >> >>>> + /* Put a q-factor only if it is less than 1.0. */ >>>> + if (q < max_q) >>>> + strbuf_addf(cached_accept_language, q_format, q); >>>> + >>>> + if (q > 1) >>>> + q--; >> >> I didn't mention this but if q ever goes below 1, wouldn't it mean >> that there is no point continuing this loop? >> -- >> To unsubscribe from this list: send the line "unsubscribe git" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > This seems to be failing under Mac OS for me > > not ok 25 - git client sends Accept-Language based on LANGUAGE, > LC_ALL, LC_MESSAGES and LANG > # > # check_language "ko-KR, *;q=0.1" ko_KR.UTF-8 de_DE.UTF-8 ja_JP.UTF-8 > en_US.UTF-8 && > # check_language "de-DE, *;q=0.1" "" de_DE.UTF-8 ja_JP.UTF-8 > en_US.UTF-8 && > # check_language "ja-JP, *;q=0.1" "" "" ja_JP.UTF-8 > en_US.UTF-8 && > # check_language "en-US, *;q=0.1" "" "" "" > en_US.UTF-8 > # verbose results Initialized empty Git repository in /Users/Shared/Jenkins/Home/jobs/git/workspace/t/trash directory.t5550-http-fetch-dumb/.git/ expecting success: git config push.default matching && echo content1 >file && git add file && git commit -m one echo content2 >file && git add file && git commit -m two [master (root-commit) f5983e9] one Author: A U Thor <author@xxxxxxxxxxx> 1 file changed, 1 insertion(+) create mode 100644 file [master 2ff8a06] two Author: A U Thor <author@xxxxxxxxxxx> 1 file changed, 1 insertion(+), 1 deletion(-) ok 1 - setup repository expecting success: cp -R .git "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && git config core.bare true && mkdir -p hooks && echo "exec git update-server-info" >hooks/post-update && chmod +x hooks/post-update && hooks/post-update ) && git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && git push public master:master Everything up-to-date ok 2 - create http-accessible bare repository with loose objects expecting success: git clone $HTTPD_URL/dumb/repo.git clone-tmpl && cp -R clone-tmpl clone && test_cmp file clone/file Cloning into 'clone-tmpl'... ok 3 - clone http repository expecting success: mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/" && cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \ "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git" ok 4 - create password-protected repository expecting success: write_script "$TRASH_DIRECTORY/askpass" <<-\EOF && echo >>"$TRASH_DIRECTORY/askpass-query" "askpass: $*" && case "$*" in *Username*) what=user ;; *Password*) what=pass ;; esac && cat "$TRASH_DIRECTORY/askpass-$what" EOF GIT_ASKPASS="$TRASH_DIRECTORY/askpass" && export GIT_ASKPASS && export TRASH_DIRECTORY ok 5 - setup askpass helper expecting success: set_askpass wrong && test_must_fail git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-fail && expect_askpass both wrong Cloning into 'clone-auth-fail'... fatal: Authentication failed for 'http://127.0.0.1:5550/auth/dumb/repo.git/' ok 6 - cloning password-protected repository can fail expecting success: set_askpass wrong && git clone "$HTTPD_URL_USER_PASS/auth/dumb/repo.git" clone-auth-none && expect_askpass none Cloning into 'clone-auth-none'... ok 7 - http auth can use user/pass in URL expecting success: set_askpass wrong pass@host && git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-pass && expect_askpass pass user@host Cloning into 'clone-auth-pass'... ok 8 - http auth can use just user in URL expecting success: set_askpass user@host pass@host && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-both && expect_askpass both user@host Cloning into 'clone-auth-both'... ok 9 - http auth can request both user and pass expecting success: test_config_global credential.helper "!f() { cat >/dev/null echo username=user@host echo password=pass@host }; f" && set_askpass wrong && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-helper && expect_askpass none Cloning into 'clone-auth-helper'... ok 10 - http auth respects credential helper config expecting success: test_config_global "credential.$HTTPD_URL.username" user@host && set_askpass wrong pass@host && git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-user && expect_askpass pass user@host Cloning into 'clone-auth-user'... ok 11 - http auth can get username from config expecting success: test_config_global "credential.$HTTPD_URL.username" wrong && set_askpass wrong pass@host && git clone "$HTTPD_URL_USER/auth/dumb/repo.git" clone-auth-user2 && expect_askpass pass user@host Cloning into 'clone-auth-user2'... ok 12 - configured username does not override URL expecting success: echo content >>file && git commit -a -m two && git push public && (cd clone && git pull) && test_cmp file clone/file [master d4af499] two Author: A U Thor <author@xxxxxxxxxxx> 1 file changed, 1 insertion(+) To /Users/Shared/Jenkins/Home/jobs/git/workspace/t/trash directory.t5550-http-fetch-dumb/httpd/www/repo.git 2ff8a06..d4af499 master -> master >From http://127.0.0.1:5550/dumb/repo 2ff8a06..d4af499 master -> origin/master Updating 2ff8a06..d4af499 Fast-forward file | 1 + 1 file changed, 1 insertion(+) ok 13 - fetch changes via http expecting success: cp -R clone-tmpl clone2 && HEAD=$(git rev-parse --verify HEAD) && (cd clone2 && git http-fetch -a -w heads/master-new $HEAD $(git config remote.origin.url) && git checkout master-new && test $HEAD = $(git rev-parse --verify HEAD)) && test_cmp file clone2/file Switched to branch 'master-new' ok 14 - fetch changes via manual http-fetch expecting success: git push public master:other && (cd clone && git remote set-head origin -d && git remote set-head origin -a && git symbolic-ref refs/remotes/origin/HEAD > output && echo refs/remotes/origin/master > expect && test_cmp expect output ) To /Users/Shared/Jenkins/Home/jobs/git/workspace/t/trash directory.t5550-http-fetch-dumb/httpd/www/repo.git * [new branch] master -> other origin/HEAD set to master ok 15 - http remote detects correct HEAD expecting success: cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && git --bare repack -a -d ) && git clone $HTTPD_URL/dumb/repo_pack.git Cloning into 'repo_pack'... ok 16 - fetch packed objects expecting success: cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && p=`ls objects/pack/pack-*.pack` && chmod u+w $p && printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc ) && mkdir repo_bad1.git && (cd repo_bad1.git && git --bare init && test_must_fail git --bare fetch $HTTPD_URL/dumb/repo_bad1.git && test 0 = `ls objects/pack/pack-*.pack | wc -l` ) 1+0 records in 1+0 records out 256 bytes transferred in 0.000014 secs (18512790 bytes/sec) Initialized empty Git repository in /Users/Shared/Jenkins/Home/jobs/git/workspace/t/trash directory.t5550-http-fetch-dumb/repo_bad1.git/ fatal: pack has bad object at offset 168: inflate returned -5 error: Unable to find d4af499a00b28bc0ab78fa94cc6a449fae19b08d under http://127.0.0.1:5550/dumb/repo_bad1.git Cannot obtain needed object d4af499a00b28bc0ab78fa94cc6a449fae19b08d error: fetch failed. ls: objects/pack/pack-*.pack: No such file or directory ok 17 - fetch notices corrupt pack expecting success: cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad2.git && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad2.git && p=`ls objects/pack/pack-*.idx` && chmod u+w $p && printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc ) && mkdir repo_bad2.git && (cd repo_bad2.git && git --bare init && test_must_fail git --bare fetch $HTTPD_URL/dumb/repo_bad2.git && test 0 = `ls objects/pack | wc -l` ) 1+0 records in 1+0 records out 256 bytes transferred in 0.000013 secs (19522579 bytes/sec) Initialized empty Git repository in /Users/Shared/Jenkins/Home/jobs/git/workspace/t/trash directory.t5550-http-fetch-dumb/repo_bad2.git/ error: non-monotonic index /Users/Shared/Jenkins/Home/jobs/git/workspace/t/trash directory.t5550-http-fetch-dumb/repo_bad2.git/objects/pack/pack-7244949d8d6e59a30923b3fff7801f159ef4ba5d.idx.temp error: Unable to find d4af499a00b28bc0ab78fa94cc6a449fae19b08d under http://127.0.0.1:5550/dumb/repo_bad2.git Cannot obtain needed object d4af499a00b28bc0ab78fa94cc6a449fae19b08d error: fetch failed. ok 18 - fetch notices corrupt idx expecting success: grep /git-upload-pack <"$HTTPD_ROOT_PATH"/access.log >act : >exp test_cmp exp act ok 19 - did not use upload-pack service expecting success: test_must_fail git clone "$HTTPD_URL/error/text" 2>stderr && grep "this is the error message" stderr remote: this is the error message ok 20 - git client shows text/plain errors expecting success: test_must_fail git clone "$HTTPD_URL/error/html" 2>stderr && ! grep "this is the error message" stderr ok 21 - git client does not show html errors expecting success: test_must_fail git clone "$HTTPD_URL/error/charset" 2>stderr && grep "this is the error message" stderr remote: this is the error message ok 22 - git client shows text/plain with a charset expecting success: test_must_fail git clone "$HTTPD_URL/error/utf16" 2>stderr && grep "this is the error message" stderr remote: this is the error message ok 23 - http error messages are reencoded expecting success: test_must_fail git clone "$HTTPD_URL/error/odd-spacing" 2>stderr && grep "this is the error message" stderr remote: this is the error message ok 24 - reencoding is robust to whitespace oddities expecting success: check_language "ko-KR, *;q=0.1" ko_KR.UTF-8 de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8 && check_language "de-DE, *;q=0.1" "" de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8 && check_language "ja-JP, *;q=0.1" "" "" ja_JP.UTF-8 en_US.UTF-8 && check_language "en-US, *;q=0.1" "" "" "" en_US.UTF-8 not ok 25 - git client sends Accept-Language based on LANGUAGE, LC_ALL, LC_MESSAGES and LANG # # check_language "ko-KR, *;q=0.1" ko_KR.UTF-8 de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8 && # check_language "de-DE, *;q=0.1" "" de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8 && # check_language "ja-JP, *;q=0.1" "" "" ja_JP.UTF-8 en_US.UTF-8 && # check_language "en-US, *;q=0.1" "" "" "" en_US.UTF-8 # expecting success: check_language "ko-KR, en-US;q=0.99, fr-CA;q=0.98, de;q=0.97, sr;q=0.96, \ ja;q=0.95, zh;q=0.94, sv;q=0.93, pt;q=0.92, nb;q=0.91, *;q=0.01" \ ko_KR.EUC-KR:en_US.UTF-8:fr_CA:de.UTF-8@euro:sr@latin:ja:zh:sv:pt:nb ok 26 - git client sends Accept-Language with many preferred languages expecting success: GIT_CURL_VERBOSE=1 LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr && ! grep "^Accept-Language:" stderr d4af499a00b28bc0ab78fa94cc6a449fae19b08d HEAD d4af499a00b28bc0ab78fa94cc6a449fae19b08d refs/heads/master d4af499a00b28bc0ab78fa94cc6a449fae19b08d refs/heads/other ok 27 - git client does not send an empty Accept-Language # failed 1 among 27 test(s) 1..27 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html