From patchwork Tue May 31 23:12:33 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12866240 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6A985C433F5 for ; Tue, 31 May 2022 23:12:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1348631AbiEaXMn (ORCPT ); Tue, 31 May 2022 19:12:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348627AbiEaXMk (ORCPT ); Tue, 31 May 2022 19:12:40 -0400 Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5BA718FFA4 for ; Tue, 31 May 2022 16:12:39 -0700 (PDT) Received: by mail-wr1-x42a.google.com with SMTP id q7so21415wrg.5 for ; Tue, 31 May 2022 16:12:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=31DnEc6JkNQXEuRRBsgwpj5jziDg3l6B6CvTP/X6o9I=; b=k4ejlva8lBKry+WPf2rxYmKVpNaICeB2fPDrBqc1psDC75WKgvO0rcMRXqhisekIw+ Y6UsNOHJJpaYo4yHcsG5IE6HrXD9fjHJgAPwr/ZlXIakV/0Hql6bK9oCsaPcwS1LLw8s HL9Z9vldgdFtNpB/iArmXxwEshK6zJmK2DqAV036u/0LXI9AUlASjhQBS73CHWumughf fVjauvBvEjtdxMUDynfTq4n1Q0cTUvXY7BD1h7wC37XS3IlGoqmjHGSY1d+4v5mNBy6y fsQ3rLNgN94yTVs4fcfapLvLYfywnXr+r/jw9MkCjKH+3HmC/WG9NdosIuw64h11+jg7 0O9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=31DnEc6JkNQXEuRRBsgwpj5jziDg3l6B6CvTP/X6o9I=; b=gMRYpU488mMALWQ3hOdEhgs6/4Qa7kt1RqBT6gqP6BUSACr21+KZwFXFw+h+zfvUBm +rxTF/1CV+Fq0ZvbdTENV2FkjMq24AACGewZEAr8b7J1N1NBzEUiTPozzyt93NMBmbNf TYusJItRu1e7eU9ZukhvekXDlmhnO3WhT0n1nRosjNZqbUQOFFn2iCnr4IoWh78ch3AS a+5OIw80EY7VKv1szo1OuMGM0BUTEaRo37DzfjA1c20zcLdiknFiRSEhLDEayI1X7nbL JlABg6wYrYjKRkRcYGbgN5C3/URS11PjKzGPE/QuGHgCJIB6uaTSDHvO34i5iTJX0/et MCng== X-Gm-Message-State: AOAM533ZqAwh1RUluD7lo+L2EBkkpmmN4YOXKUsoTHRW2fqf6fKfUxv2 Yp2hv/EdaGNGrc9xvhvV4gfHg0SwuUI= X-Google-Smtp-Source: ABdhPJwpdb0o7r12GPptKh8gnEkbVxKrnrfiDj7GwMkd6LkBMXMKUyEFL0ssvkB7d0RWibK5bbSxAA== X-Received: by 2002:a5d:54cc:0:b0:210:3d9f:4770 with SMTP id x12-20020a5d54cc000000b002103d9f4770mr808662wrv.122.1654038757556; Tue, 31 May 2022 16:12:37 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id be5-20020a05600c1e8500b003942a244ee6sm90839wmb.43.2022.05.31.16.12.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 May 2022 16:12:36 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Tue, 31 May 2022 23:12:33 +0000 Subject: [PATCH 1/2] remote.c: don't BUG() on 0-length branch names Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , "Ing. Martin Prantl Ph.D." , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo 4a2dcb1a08 (remote: die if branch is not found in repository, 2021-11-17) introduced a regression where multiple config entries with an empty branch name, e.g. [branch ""] remote = foo merge = bar could cause Git to fail when it tries to look up branch tracking information. We parse the config key to get (branch name, branch name length), but when the branch name subsection is empty, we get a bogus branch name, e.g. "branch..remote" gives (".remote", 0). We continue to use the bogus branch name as if it were valid, and prior to 4a2dcb1a08, this wasn't an issue because length = 0 caused the branch name to effectively be "" everywhere. However, that commit handles length = 0 inconsistently when we create the branch: - When find_branch() is called to check if the branch exists in the branch hash map, it interprets a length of 0 to mean that it should call strlen on the char pointer. - But the code path that inserts into the branch hash map interprets a length of 0 to mean that the string is 0-length. This results in the bug described above: - "branch..remote" looks for ".remote" in the branch hash map. Since we do not find it, we insert the "" entry into the hash map. - "branch..merge" looks for ".merge" in the branch hash map. Since we do not find it, we again try to insert the "" entry into the hash map. However, the entries in the branch hash map are supposed to be appended to, not overwritten. - Since overwriting an entry is a BUG(), Git fails instead of silently ignoring the empty branch name. Fix the bug by removing the convenience strlen functionality, so that 0 means that the string is 0-length. We still insert a bogus branch name into the hash map, but this will be fixed in a later commit. Reported-by: "Ing. Martin Prantl Ph.D." Helped-by: Jeff King Signed-off-by: Glen Choo --- remote.c | 6 ++---- t/t5516-fetch-push.sh | 10 ++++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/remote.c b/remote.c index 42a4e7106e1..cf7015ae8ab 100644 --- a/remote.c +++ b/remote.c @@ -195,9 +195,6 @@ static struct branch *find_branch(struct remote_state *remote_state, struct branches_hash_key lookup; struct hashmap_entry lookup_entry, *e; - if (!len) - len = strlen(name); - lookup.str = name; lookup.len = len; hashmap_entry_init(&lookup_entry, memhash(name, len)); @@ -214,7 +211,8 @@ static void die_on_missing_branch(struct repository *repo, { /* branch == NULL is always valid because it represents detached HEAD. */ if (branch && - branch != find_branch(repo->remote_state, branch->name, 0)) + branch != find_branch(repo->remote_state, branch->name, + strlen(branch->name))) die("branch %s was not found in the repository", branch->name); } diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 4dfb080433e..a05268952e9 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -598,6 +598,16 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' ' check_push_result two_repo $the_commit heads/main ' +test_expect_success 'push ignores empty branch name entries' ' + mk_test one_repo heads/main && + test_config remote.one.url one_repo && + test_config branch..remote one && + test_config branch..merge refs/heads/ && + test_config branch.main.remote one && + test_config branch.main.merge refs/heads/main && + git push +' + test_expect_success 'push with dry-run' ' mk_test testrepo heads/main && From patchwork Tue May 31 23:12:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12866241 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C02FCC433EF for ; Tue, 31 May 2022 23:12:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1348638AbiEaXMo (ORCPT ); Tue, 31 May 2022 19:12:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46374 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348629AbiEaXMl (ORCPT ); Tue, 31 May 2022 19:12:41 -0400 Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7CEB58FF9C for ; Tue, 31 May 2022 16:12:40 -0700 (PDT) Received: by mail-wr1-x430.google.com with SMTP id q7so21456wrg.5 for ; Tue, 31 May 2022 16:12:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=3ibAYxxmwqdF47KI1epRuVZR6ZM02+XIW9ONctatoZk=; b=L2P2CttkPIPJNCGM40Y5NwHqQijDaxuqGBDUIRQHZe/rN4WVyuQlFGkDMoLNIMt9Cj Q00Fy1pgcpmRjXe3gsz2ZR7aIHFBQpArnDRq9uEqa0UwQygV0+Vhec3VBVDV155rDEVx Z4+xQYp/MXnt4M1mdOvcFtH8qqbGhzXm4KDzCUA0R/nqbM8rySLtKVR8fI9RceXN13nI IEJ8v7WGbQPadhIHGGWC8EJ3VVtOSNeS3720tQBoa0AMn+VMyezwXOFYsSBUXBwfkPQ0 TbwIIgTRLP0YGSFsjmyCy0Ln9lXn+pF/vWHXHeUwxPouUdltbUZzUxaW+MUN8GRRklYe HtIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=3ibAYxxmwqdF47KI1epRuVZR6ZM02+XIW9ONctatoZk=; b=C56JGQy08gaQfDm0CPCCn7wObMusTEwdVII2cwTZPPXYmHkWRP63PGz9UfJUQFj0UA 3MrJZn+FmUiak4lplyMOjVE2kNGk0V4LinrPV9UgHKaZSZbZDgFMDzEyJ1949dnVSsoM pKEkPhfIF4oU/ELgbN5Y9RakOnBuEGSDOBAHIyz/pmSfjAm1aOeRGULrZ25v18BfpLEQ qEFC1qhHuJlA+VmJ64kuGOd3cnrL0wLjznkoF+lvpwz56FvGWxVNGUoXsCFn2vQUIql1 xoFEcdUgrPbO1uvxAUmEGUK22XHnryyw65VjlN/Pt7MBwSO29u1DjEL7yQG6v781amZc F0fA== X-Gm-Message-State: AOAM531k+mZicDNJJ9BL6P3P+NJIt6HQbVRmSJVdCcpwUnyEDxhNPhWj Rqb/ua3LiaGfxKtcExPMIwHFIixlsBk= X-Google-Smtp-Source: ABdhPJzQrMstw27rXp3YLCN8hZyeLVQovEHL4Z3btGXsBMlULc3rDtmiqdsGv4o8ehkE8I8wXiK6WA== X-Received: by 2002:a05:6000:1e0a:b0:210:32e1:3b03 with SMTP id bj10-20020a0560001e0a00b0021032e13b03mr11256766wrb.642.1654038758796; Tue, 31 May 2022 16:12:38 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l9-20020a7bc349000000b0039746638d6esm102739wmj.33.2022.05.31.16.12.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 May 2022 16:12:38 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Tue, 31 May 2022 23:12:34 +0000 Subject: [PATCH 2/2] remote.c: reject 0-length branch names Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , "Ing. Martin Prantl Ph.D." , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Branch names can't be empty, so config keys with an empty branch name, e.g. "branch..remote", are silently ignored. Since these config keys will never be useful, make it a fatal error when remote.c finds a key that starts with "branch." and has an empty subsection. Signed-off-by: Glen Choo --- remote.c | 4 ++++ t/t5516-fetch-push.sh | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/remote.c b/remote.c index cf7015ae8ab..a3888dd789c 100644 --- a/remote.c +++ b/remote.c @@ -352,8 +352,12 @@ static int handle_config(const char *key, const char *value, void *cb) struct remote_state *remote_state = cb; if (parse_config_key(key, "branch", &name, &namelen, &subkey) >= 0) { + /* There is no subsection. */ if (!name) return 0; + /* There is a subsection, but it is empty. */ + if (!namelen) + return -1; branch = make_branch(remote_state, name, namelen); if (!strcmp(subkey, "remote")) { return git_config_string(&branch->remote_name, key, value); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index a05268952e9..e99c31f8c35 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -598,13 +598,23 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' ' check_push_result two_repo $the_commit heads/main ' -test_expect_success 'push ignores empty branch name entries' ' +test_expect_success 'push rejects empty branch name entries' ' mk_test one_repo heads/main && test_config remote.one.url one_repo && test_config branch..remote one && test_config branch..merge refs/heads/ && test_config branch.main.remote one && test_config branch.main.merge refs/heads/main && + test_must_fail git push 2>err && + grep "bad config variable .branch\.\." err +' + +test_expect_success 'push ignores "branch." config without subsection' ' + mk_test one_repo heads/main && + test_config remote.one.url one_repo && + test_config branch.autoSetupMerge true && + test_config branch.main.remote one && + test_config branch.main.merge refs/heads/main && git push '