From patchwork Wed Jul 31 13:40:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Couder X-Patchwork-Id: 13748813 Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D71CD1B0136 for ; Wed, 31 Jul 2024 13:40:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.50 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433241; cv=none; b=QhIEu07Yb7mT3wrLqPD1eF9EC56JOoDshl2hTjJkkoQEseXqXeCbx1jBlmBVrwoNpO18eKqnKY3llB51HfUgYEwsu3RBu0Z+5CdWIMp6HXDs5o114czzfOCTFGvz6OigjL1ttxtK3qlXMTnKtjXTLpMUOZ5lf+R19foB1ifjZC0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433241; c=relaxed/simple; bh=inQhiDsZ0H9gi97ogwBjxug7M3qeW2t+atYHE4WdQ6g=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Ck/sMI54H0br/XSSgXKw9YmKWPutTgFexA/IJkdiGHk6xJUTrpLWbgFLAvAAtrVNi1qhZiQt4QUVk/cg3KXVywrQVP8yIb3PfhJHYw/U7/QGo/3k17q33EamFw7KfUIt6h+qhFhfHKpEKMjsejWUN1cOaxWDHOwVSd3i0UuE7b8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=evp2jB+Q; arc=none smtp.client-ip=209.85.167.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="evp2jB+Q" Received: by mail-lf1-f50.google.com with SMTP id 2adb3069b0e04-52efc60a6e6so9163173e87.1 for ; Wed, 31 Jul 2024 06:40:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722433237; x=1723038037; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=WuF3fmx0tQS7Jo2Nthk5t8HYgW9Wp3lOo6Sg4ynamPU=; b=evp2jB+Q2tVw700tB65qJN3asnoc5fYvTyK1TFhdxhBm4R861Asi7wkR0oaUPQwbVs +BzrQP8L36gc5JfJYNtAL82A7AeZ+y10uIS2jG7aJWFmd/3HT4o61yVPW3ak5DjHcOY4 pSU1/og5hwb9Gvgehcl+zwkwVRVSXjVN8VnHyqWQYjYi0nKsJjruzSHZRMA/woOQNJbc jYaGXv1f/Z9H1HXRwKD4ykEc/ENJZrP7jpKywMu9N1cT2bd+GtU+SAKy6LA/pvuMYY0I +gAEMxPuao1C/XDuIUu/icef9zFBOKtPC0F/oBVjn3ujarPZHMdBKHy6E/XRPiIOZmSW 33Kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722433237; x=1723038037; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WuF3fmx0tQS7Jo2Nthk5t8HYgW9Wp3lOo6Sg4ynamPU=; b=tqh02EnEaajtm2vo+Ha2TkfOe2rQvhgbq0PO+2+O7O0IeYITfWH08wMfev8eCInnIL 7yKVyrpMb0LDC6fMr3XhsRDZ1OE0h5ty41dk5pz3jItlYRusDQ9p2ebtdWuSXlD4Rk1B 3uGUPA3kpQutoPyD6ahRU2zH4m9pQvjo1fAYJViR4xJgGEn6zWLI11j5ydn+uMbdVH9s iZ6p4tJ1nSLaItww0tTL5I+EfTCYilTTQtBw1qA17Z9BzwudGjUYy1l3ECg73Mz0AvmZ ktQuqYpWGQ04YbA56M7sad6s73gDvqDa+z1NhgN7wQEnAq1N+5Z7cD9lBn8/yOI8a8/G DMZw== X-Gm-Message-State: AOJu0Yz+l4O8d5x5kxevEeksV7/Faomq9WkMxC7r+d+oD2wu7ZPs3iM0 pOHLJ1nEgEL2yIROmgGebmcoDpEVvB23wpUH0Vd+FjsFCdn5U1pUX0zHJg== X-Google-Smtp-Source: AGHT+IF3r9TBZ6CXJwhKGbhPFVxF8uQu7OqFd53euzLw6nsekgVu3q68pySyZbbHkr6djoh4JyUcng== X-Received: by 2002:a05:6512:3e08:b0:52e:bdfc:1d09 with SMTP id 2adb3069b0e04-5309b26c090mr9814816e87.18.1722433236959; Wed, 31 Jul 2024 06:40:36 -0700 (PDT) Received: from christian-Precision-5550.. (176-138-135-207.abo.bbox.fr. [176.138.135.207]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5ac63c50fdesm8669641a12.56.2024.07.31.06.40.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jul 2024 06:40:36 -0700 (PDT) From: Christian Couder To: git@vger.kernel.org Cc: Junio C Hamano , John Cai , Patrick Steinhardt , Christian Couder , Christian Couder Subject: [PATCH 1/4] version: refactor strbuf_sanitize() Date: Wed, 31 Jul 2024 15:40:11 +0200 Message-ID: <20240731134014.2299361-2-christian.couder@gmail.com> X-Mailer: git-send-email 2.46.0.4.gbcb884ee16 In-Reply-To: <20240731134014.2299361-1-christian.couder@gmail.com> References: <20240731134014.2299361-1-christian.couder@gmail.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The git_user_agent_sanitized() function performs some sanitizing to avoid special characters being sent over the line and possibly messing up with the protocol or with the parsing on the other side. Let's extract this sanitizing into a new strbuf_sanitize() function, as we will want to reuse it in a following patch, and let's put it into strbuf.{c,h}. While at it, let's also make a few small improvements: - use 'size_t' for 'i' instead of 'int', - move the declaration of 'i' inside the 'for ( ... )', - use strbuf_detach() to explicitely detach the string contained by the 'sb' strbuf. Signed-off-by: Christian Couder --- strbuf.c | 9 +++++++++ strbuf.h | 7 +++++++ version.c | 9 ++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/strbuf.c b/strbuf.c index 3d2189a7f6..cccfdec0e3 100644 --- a/strbuf.c +++ b/strbuf.c @@ -1082,3 +1082,12 @@ void strbuf_strip_file_from_path(struct strbuf *sb) char *path_sep = find_last_dir_sep(sb->buf); strbuf_setlen(sb, path_sep ? path_sep - sb->buf + 1 : 0); } + +void strbuf_sanitize(struct strbuf *sb) +{ + strbuf_trim(sb); + for (size_t i = 0; i < sb->len; i++) { + if (sb->buf[i] <= 32 || sb->buf[i] >= 127) + sb->buf[i] = '.'; + } +} diff --git a/strbuf.h b/strbuf.h index 003f880ff7..884157873e 100644 --- a/strbuf.h +++ b/strbuf.h @@ -664,6 +664,13 @@ typedef int (*char_predicate)(char ch); void strbuf_addstr_urlencode(struct strbuf *sb, const char *name, char_predicate allow_unencoded_fn); +/* + * Trim and replace each character with ascii code below 32 or above + * 127 (included) using a dot '.' character. Useful for sending + * capabilities. + */ +void strbuf_sanitize(struct strbuf *sb); + __attribute__((format (printf,1,2))) int printf_ln(const char *fmt, ...); __attribute__((format (printf,2,3))) diff --git a/version.c b/version.c index 41b718c29e..951e6dca74 100644 --- a/version.c +++ b/version.c @@ -24,15 +24,10 @@ const char *git_user_agent_sanitized(void) if (!agent) { struct strbuf buf = STRBUF_INIT; - int i; strbuf_addstr(&buf, git_user_agent()); - strbuf_trim(&buf); - for (i = 0; i < buf.len; i++) { - if (buf.buf[i] <= 32 || buf.buf[i] >= 127) - buf.buf[i] = '.'; - } - agent = buf.buf; + strbuf_sanitize(&buf); + agent = strbuf_detach(&buf, NULL); } return agent; From patchwork Wed Jul 31 13:40:12 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Couder X-Patchwork-Id: 13748814 Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DF8591B29AF for ; Wed, 31 Jul 2024 13:40:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.48 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433243; cv=none; b=uvBsFWs/njPulwiUtgwnL4S2sWsKdVZWuAJ/QrdNX8w72c/iJqJxPXtMyWm+ceUpn1DzaCb0pUNWg0QD7EIka57aug7KaZkcx2arkiv6mG3mRjESelvZK4KkJ2H6q8TaekoPScFVK7cLJ5IAar8opzFOHH2ht4NVrud2T2OxK9o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433243; c=relaxed/simple; bh=Gng4k46f2LffGGnECwQtmZwK7f3IGq4hSi1EkConw+Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=uyDhbA+GzoB48JtUhxyGj8d0FWg/zIh6+4TtC0Ukwi2wJQM9emf2HQBOK2Wv9eeNDNFYLfxJjynW1mfT9LHlBwMuvMp+FHfIW1M/Ue11tV1P/Co7rgrLx9TL1OEJX1pm7EMjG00f1ogqnQB94Yeokr6zVn1vwTWfs3KQnT0fKx8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=bzsmii4q; arc=none smtp.client-ip=209.85.208.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="bzsmii4q" Received: by mail-ed1-f48.google.com with SMTP id 4fb4d7f45d1cf-5a15692b6f6so8957135a12.0 for ; Wed, 31 Jul 2024 06:40:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722433240; x=1723038040; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vUtbEsMeijDKXZNMoKbWZEmXR9Kf9ouXo3brZom4o4g=; b=bzsmii4qn7n8HYgaH1MGuEu5iVxtiPlldEwwr0LIGLwVq1UEQcq/hgtr7NAmy170V2 Jh5hGgzkZrmUOvrzLwC4CzrNu7O3Eq2a0wPf9Wk2uVVOwRrt5sfkgZfi3RbUXNPU9kMu 1s+blkzZqmWSsda8ygiyZ+prMQlObNNMO7SoU90D3n2u+GlXT1aa3ofNPAK/s+alaHfp ntxe41MyQrhhoUQfRxOi9iEfEUHX4UW6IFBso0iL8irvM6+GQSXrPMg9w7E4jYMlm1/b pZV2iTmKyG/5n0iRddu7nYbmnZPyyIKVyrTD2X3jWrP0nPQndCC1HjwbQ5Aucc3Vt183 ulKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722433240; x=1723038040; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vUtbEsMeijDKXZNMoKbWZEmXR9Kf9ouXo3brZom4o4g=; b=mhcYIJP5Dooa5XzpljKZvVgMne9Y/SKEUb5l5jz46GB4WiLZpakqQSL1pqKKqUfDrg s1Ds3YBChEoCjF4wIYSFf4aaMnnqfY8J1wMWDV7oeS7nnTI5Tcy6tILHM2Y73rooWx3h mR3ngLFQnWCZoqzSMKa+/imdDx5GdfIdmCF5HnSxB5V8SUYt/QKoG/RPq95IdNt/+CZv WtBj0AFH9jrQ+D2myoaB2Mpg47pOxC1edcQ1SjFosxvPZmWCkAy7MnxgUahXLvr6dwie xmAKGUtyV4qrRz7BD3m/sQwqs5T/EDa81NGfoooq5eI33y8mp0QeLt7RCMr7kB8yxcXb dq6A== X-Gm-Message-State: AOJu0YzgjnYTIjF8wEPi2/OqFh0HqZQrY021GUg0iO7SmdQJO7MQDGWg FmeYCUAX0eQyoM4+Zy4kSlvdo++vupNUnK0yhX1kcU7jIvw2dRIWFmsn1g== X-Google-Smtp-Source: AGHT+IFK+sLmudskAI0LnSCxBfvKzv5VXpdvDAz+a/lqE9OghuBvzF2xxqQQfT7DUjSDUpu7XBBW3A== X-Received: by 2002:a50:baa9:0:b0:57c:7471:a0dd with SMTP id 4fb4d7f45d1cf-5b0206ce157mr8848631a12.12.1722433239082; Wed, 31 Jul 2024 06:40:39 -0700 (PDT) Received: from christian-Precision-5550.. (176-138-135-207.abo.bbox.fr. [176.138.135.207]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5ac63c50fdesm8669641a12.56.2024.07.31.06.40.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jul 2024 06:40:37 -0700 (PDT) From: Christian Couder To: git@vger.kernel.org Cc: Junio C Hamano , John Cai , Patrick Steinhardt , Christian Couder , Christian Couder Subject: [PATCH 2/4] strbuf: refactor strbuf_trim_trailing_ch() Date: Wed, 31 Jul 2024 15:40:12 +0200 Message-ID: <20240731134014.2299361-3-christian.couder@gmail.com> X-Mailer: git-send-email 2.46.0.4.gbcb884ee16 In-Reply-To: <20240731134014.2299361-1-christian.couder@gmail.com> References: <20240731134014.2299361-1-christian.couder@gmail.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 We often have to split strings at some specified terminator character. The strbuf_split*() functions, that we can use for this purpose, return substrings that include the terminator character, so we often need to remove that character. When it is a whitespace, newline or directory separator, the terminator character can easily be removed using an existing triming function like strbuf_rtrim(), strbuf_trim_trailing_newline() or strbuf_trim_trailing_dir_sep(). There is no function to remove that character when it's not one of those characters though. Let's introduce a new strbuf_trim_trailing_ch() function that can be used to remove any trailing character, and let's refactor existing code that manually removed trailing characters using this new function. We are also going to use this new function in a following commit. Signed-off-by: Christian Couder --- strbuf.c | 7 +++++++ strbuf.h | 3 +++ trace2/tr2_cfg.c | 10 ++-------- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/strbuf.c b/strbuf.c index cccfdec0e3..c986ec28f4 100644 --- a/strbuf.c +++ b/strbuf.c @@ -134,6 +134,13 @@ void strbuf_trim_trailing_dir_sep(struct strbuf *sb) sb->buf[sb->len] = '\0'; } +void strbuf_trim_trailing_ch(struct strbuf *sb, int c) +{ + while (sb->len > 0 && sb->buf[sb->len - 1] == c) + sb->len--; + sb->buf[sb->len] = '\0'; +} + void strbuf_trim_trailing_newline(struct strbuf *sb) { if (sb->len > 0 && sb->buf[sb->len - 1] == '\n') { diff --git a/strbuf.h b/strbuf.h index 884157873e..5e389ab065 100644 --- a/strbuf.h +++ b/strbuf.h @@ -197,6 +197,9 @@ void strbuf_trim_trailing_dir_sep(struct strbuf *sb); /* Strip trailing LF or CR/LF */ void strbuf_trim_trailing_newline(struct strbuf *sb); +/* Strip trailing character c */ +void strbuf_trim_trailing_ch(struct strbuf *sb, int c); + /** * Replace the contents of the strbuf with a reencoded form. Returns -1 * on error, 0 on success. diff --git a/trace2/tr2_cfg.c b/trace2/tr2_cfg.c index d96d908bb9..356fcd38f4 100644 --- a/trace2/tr2_cfg.c +++ b/trace2/tr2_cfg.c @@ -33,10 +33,7 @@ static int tr2_cfg_load_patterns(void) tr2_cfg_patterns = strbuf_split_buf(envvar, strlen(envvar), ',', -1); for (s = tr2_cfg_patterns; *s; s++) { - struct strbuf *buf = *s; - - if (buf->len && buf->buf[buf->len - 1] == ',') - strbuf_setlen(buf, buf->len - 1); + strbuf_trim_trailing_ch(*s, ','); strbuf_trim_trailing_newline(*s); strbuf_trim(*s); } @@ -72,10 +69,7 @@ static int tr2_load_env_vars(void) tr2_cfg_env_vars = strbuf_split_buf(varlist, strlen(varlist), ',', -1); for (s = tr2_cfg_env_vars; *s; s++) { - struct strbuf *buf = *s; - - if (buf->len && buf->buf[buf->len - 1] == ',') - strbuf_setlen(buf, buf->len - 1); + strbuf_trim_trailing_ch(*s, ','); strbuf_trim_trailing_newline(*s); strbuf_trim(*s); } From patchwork Wed Jul 31 13:40:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Couder X-Patchwork-Id: 13748815 Received: from mail-ed1-f45.google.com (mail-ed1-f45.google.com [209.85.208.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 30DB21B3725 for ; Wed, 31 Jul 2024 13:40:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433245; cv=none; b=A6Rbkj3y+VLLRPlBHA00RJjUtHvrivuHoyIh2/Zk0CzU4n3HvVFII1Nq62h96MRF/905GFaiF5JkPD7hlNH/uIzOaDaXz+0iZDYlAq0hdRUtJY/dw8Z3J5LJropUszeTP11qAu4jgVgO5sUD4tiIN/aHN3V6iq7vJmB3BxWAASQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433245; c=relaxed/simple; bh=hJlwTA+qPz1rgVod0RhTPKeC1xQyJlfg7Gw9/FmjAfw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=bMgXqt37JgV1uf4OEnW+Yip1oxmY1C8ccaB/ISiFv/LNOl/SrnPXpHGAkbLKXdHKoyhwRn5GWztZ1+pS8T6eJs3slvSYWoyboWQnG26QoeGI37JWNtmxICbMuixtbq3e6M/jXO57A5uubLhVumOu92iuyKGdFZesNJ0NKQa4Hrw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=mQp5NYN7; arc=none smtp.client-ip=209.85.208.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mQp5NYN7" Received: by mail-ed1-f45.google.com with SMTP id 4fb4d7f45d1cf-595856e2336so1668279a12.1 for ; Wed, 31 Jul 2024 06:40:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722433241; x=1723038041; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=/eR2ppw8yqKUtBKcBi0eacxbKntUQ9fIW8hQN9g+P2c=; b=mQp5NYN7oPWfjD4PaPFXFGMeXQkr1XKNcavRLnCDuaVb/65C9Uc3vTHxcKM2gpg0ld koF2kwRV9VwZHx3ohrZLwnjxQ+xUfEqKKFCcEOKzTFFAzuupQFNxhS04ZwgNfTecJ4C0 PfKb15mPDvWl1ispEAHCppX0ZLe0KACA2MU5ys6euPyqwnChk9VKQQIhsCmZdsGgXna5 neEcUSZBL0TZJ0wDULpObqkC4UdL1V77ybJpdezUlCRDnnaQJ0JW86VuRPrJlOJ3Z1Pn Rf7TldDqoRdA3rQ7UTlGInc+yHqwz6o5QCejG7n1ZibioUinq+cgVdGxfFuAODVxDzJB J4uQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722433241; x=1723038041; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/eR2ppw8yqKUtBKcBi0eacxbKntUQ9fIW8hQN9g+P2c=; b=W9YCUqW27bThZMWQtT+W9BePaI25pirOJY5jMd7rLlN4cBcKmvVU7JMsFEkMTLvbdD rCVFQSmr8/pqoXVQSdzb7Aa34tNQamjUWifC41rbx/S7hbJTBsA26oVN+8k/eXnxwfTv jfobpx3V2of4X2pQNaxseBkljVUA1YwQlBUspkbrx768fKbdoT2Twn2SzlERLbxyujAm /l8F9g0+yeamf2QxsaCsHfaoMcuRSOOx62O81yBeBjEbntPcYQaf6JUFXbVWg2+ePzrf /3OD/9NiT/2GUULvZUBkZuHki8sFdfTO5VgVMMO3XiMVkGdsWmN0uzo2KdV9yW/J5pot CSzg== X-Gm-Message-State: AOJu0YxFJGLl3PJkLCavGK66V2H8ALLRZWULFHqqn6mSAOn+YLGP79O7 15unQgmk3s7O2R0RXa9efJ0R14Cyf6pd0wwk8ru7RL6RBlxxroOnaTyAfg== X-Google-Smtp-Source: AGHT+IGqjv+5ngDBr06iNWYLBTCne2Aolf+putPkzh/rU6p172QiN3N3TduY6hXtUc3zr7IRab2mxA== X-Received: by 2002:a50:c30f:0:b0:58c:b2b8:31b2 with SMTP id 4fb4d7f45d1cf-5b46cf13951mr4552624a12.17.1722433240149; Wed, 31 Jul 2024 06:40:40 -0700 (PDT) Received: from christian-Precision-5550.. (176-138-135-207.abo.bbox.fr. [176.138.135.207]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5ac63c50fdesm8669641a12.56.2024.07.31.06.40.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jul 2024 06:40:39 -0700 (PDT) From: Christian Couder To: git@vger.kernel.org Cc: Junio C Hamano , John Cai , Patrick Steinhardt , Christian Couder , Christian Couder Subject: [PATCH 3/4] Add 'promisor-remote' capability to protocol v2 Date: Wed, 31 Jul 2024 15:40:13 +0200 Message-ID: <20240731134014.2299361-4-christian.couder@gmail.com> X-Mailer: git-send-email 2.46.0.4.gbcb884ee16 In-Reply-To: <20240731134014.2299361-1-christian.couder@gmail.com> References: <20240731134014.2299361-1-christian.couder@gmail.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 When a server repository S borrows some objects from a promisor remote X, then a client repository C which would like to clone or fetch from S might, or might not, want to also borrow objects from X. Also S might, or might not, want to advertise X as a good way for C to directly get objects from, instead of C getting everything through S. To allow S and C to agree on C using X or not, let's introduce a new "promisor-remote" capability in the protocol v2, as well as a few new configuration variables: - "promisor.advertise" on the server side, and: - "promisor.acceptFromServer" on the client side. By default, or if "promisor.advertise" is set to 'false', a server S will advertise only the "promisor-remote" capability without passing any argument through this capability. This means that S supports the new capability but doesn't wish any client C to directly access any promisor remote X S might use. If "promisor.advertise" is set to 'true', S will advertise its promisor remotes with a string like: promisor-remote=[;]... where each element contains information about a single promisor remote in the form: name=[,url=] where is the name of a promisor remote and is the urlencoded url of the promisor remote named . For now, the URL is passed in addition to the name. In the future, it might be possible to pass other information like a filter-spec that the client should use when cloning from S, or a token that the client should use when retrieving objects from X. It might also be possible in the future for "promisor.advertise" to have other values like "onlyName", so that no URL is advertised. By default or if "promisor.acceptFromServer" is set to "None", the client will not accept to use the promisor remotes that might have been advertised by the server. In this case, the client will advertise only "promisor-remote" in its reply to the server. This means that the client has the "promisor-remote" capability but decided not to use any of the promisor remotes that the server might have advertised. If "promisor.acceptFromServer" is set to "All", on the contrary, the client will accept to use all the promisor remotes that the server advertised and it will reply with a string like: promisor-remote=[;]... where the elements are the names of all the promisor remotes the server advertised. If the server advertised no promisor remote though, the client will reply with just "promisor-remote". In a following commit, other values for "promisor.acceptFromServer" will be implemented so that the client will be able to decide the promisor remotes it accepts depending on the name and URL it received from the server. So even if that name and URL information is not used much right now, it will be needed soon. Signed-off-by: Christian Couder --- Documentation/config/promisor.txt | 13 ++ Documentation/gitprotocol-v2.txt | 37 ++++++ connect.c | 7 + promisor-remote.c | 182 ++++++++++++++++++++++++++ promisor-remote.h | 26 +++- serve.c | 21 +++ t/t5555-http-smart-common.sh | 1 + t/t5701-git-serve.sh | 1 + t/t5710-promisor-remote-capability.sh | 124 ++++++++++++++++++ upload-pack.c | 3 + 10 files changed, 414 insertions(+), 1 deletion(-) create mode 100755 t/t5710-promisor-remote-capability.sh diff --git a/Documentation/config/promisor.txt b/Documentation/config/promisor.txt index 98c5cb2ec2..e3939d83a9 100644 --- a/Documentation/config/promisor.txt +++ b/Documentation/config/promisor.txt @@ -1,3 +1,16 @@ promisor.quiet:: If set to "true" assume `--quiet` when fetching additional objects for a partial clone. + +promisor.advertise:: + If set to "true", a server will use the "promisor-remote" + capability, see linkgit:gitprotocol-v2[5], to advertise the + promisor remotes it is using if any. Default is "false", which + means no promisor remote is advertised. + +promisor.acceptFromServer:: + If set to "all", a client will accept all the promisor remotes + a server might advertise using the "promisor-remote" + capability, see linkgit:gitprotocol-v2[5]. Default is "none", + which means no promisor remote advertised by a server will be + accepted. diff --git a/Documentation/gitprotocol-v2.txt b/Documentation/gitprotocol-v2.txt index 414bc625d5..4d8d3839c4 100644 --- a/Documentation/gitprotocol-v2.txt +++ b/Documentation/gitprotocol-v2.txt @@ -781,6 +781,43 @@ retrieving the header from a bundle at the indicated URI, and thus save themselves and the server(s) the request(s) needed to inspect the headers of that bundle or bundles. +promisor-remote= +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The server may advertise some promisor remotes it is using, if it's OK +for the server that a client uses them too. In this case +should be of the form: + + pr-infos = pr-info | pr-infos ";" pr-info + + pr-info = "name=" pr-name | "name=" pr-name "," "url=" pr-url + +where `pr-name` is the name of a promisor remote, and `pr-url` the +urlencoded URL of that promisor remote. + +In this case a client wanting to use one or more promisor remotes the +server advertised should reply with "promisor-remote=" where + should be of the form: + + pr-names = pr-name | pr-names ";" pr-name + +where `pr-name` is the name of a promisor remote the server +advertised. + +If the server prefers a client not to use any promisor remote the +server uses, or if the server doesn't use any promisor remote, it +should only advertise "promisor-remote" without any value or "=" sign +after it. + +In this case, or if the client doesn't want to use any promisor remote +the server advertised, the client should reply only "promisor-remote" +without any value or "=" sign after it. + +The "promisor.advertise" and "promisor.acceptFromServer" configuration +options can be used on the server and client side respectively to +control what they advertise or accept respectively. See the +documentation of these configuration options for more information. + GIT --- Part of the linkgit:git[1] suite diff --git a/connect.c b/connect.c index cf84e631e9..284ea3cf12 100644 --- a/connect.c +++ b/connect.c @@ -20,6 +20,7 @@ #include "protocol.h" #include "alias.h" #include "bundle-uri.h" +#include "promisor-remote.h" static char *server_capabilities_v1; static struct strvec server_capabilities_v2 = STRVEC_INIT; @@ -485,6 +486,7 @@ void check_stateless_delimiter(int stateless_rpc, static void send_capabilities(int fd_out, struct packet_reader *reader) { const char *hash_name; + const char *promisor_remote_info; if (server_supports_v2("agent")) packet_write_fmt(fd_out, "agent=%s", git_user_agent_sanitized()); @@ -498,6 +500,11 @@ static void send_capabilities(int fd_out, struct packet_reader *reader) } else { reader->hash_algo = &hash_algos[GIT_HASH_SHA1]; } + if (server_feature_v2("promisor-remote", &promisor_remote_info)) { + char *reply = promisor_remote_reply(promisor_remote_info); + packet_write_fmt(fd_out, "promisor-remote%s", reply ? reply : ""); + free(reply); + } } int get_remote_bundle_uri(int fd_out, struct packet_reader *reader, diff --git a/promisor-remote.c b/promisor-remote.c index 317e1b127f..d347f4d9b5 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -11,6 +11,7 @@ #include "strvec.h" #include "packfile.h" #include "environment.h" +#include "url.h" struct promisor_remote_config { struct promisor_remote *promisors; @@ -219,6 +220,18 @@ int repo_has_promisor_remote(struct repository *r) return !!repo_promisor_remote_find(r, NULL); } +int repo_has_accepted_promisor_remote(struct repository *r) +{ + struct promisor_remote *p; + + promisor_remote_init(r); + + for (p = r->promisor_remote_config->promisors; p; p = p->next) + if (p->accepted) + return 1; + return 0; +} + static int remove_fetched_oids(struct repository *repo, struct object_id **oids, int oid_nr, int to_free) @@ -290,3 +303,172 @@ void promisor_remote_get_direct(struct repository *repo, if (to_free) free(remaining_oids); } + +static int allow_unsanitized(char ch) +{ + if (ch == ',' || ch == ';' || ch == '%') + return 0; + return ch > 32 && ch < 127; +} + +static void promisor_info_vecs(struct repository *repo, + struct strvec *names, + struct strvec *urls) +{ + struct promisor_remote *r; + + promisor_remote_init(repo); + + for (r = repo->promisor_remote_config->promisors; r; r = r->next) { + char *url; + char *url_key = xstrfmt("remote.%s.url", r->name); + + strvec_push(names, r->name); + strvec_push(urls, git_config_get_string(url_key, &url) ? NULL : url); + + free(url); + free(url_key); + } +} + +void promisor_remote_info(struct repository *repo, struct strbuf *buf) +{ + struct strbuf sb = STRBUF_INIT; + int advertise_promisors = 0; + struct strvec names = STRVEC_INIT; + struct strvec urls = STRVEC_INIT; + + git_config_get_bool("promisor.advertise", &advertise_promisors); + + if (!advertise_promisors) + return; + + promisor_info_vecs(repo, &names, &urls); + + for (size_t i = 0; i < names.nr; i++) { + if (sb.len) + strbuf_addch(&sb, ';'); + strbuf_addf(&sb, "name=%s", names.v[i]); + if (urls.v[i]) { + strbuf_addstr(&sb, ",url="); + strbuf_addstr_urlencode(&sb, urls.v[i], allow_unsanitized); + } + } + + strbuf_sanitize(&sb); + strbuf_addbuf(buf, &sb); + + strvec_clear(&names); + strvec_clear(&urls); +} + +enum accept_promisor { + ACCEPT_NONE = 0, + ACCEPT_ALL +}; + +static int should_accept_remote(enum accept_promisor accept, + const char *remote_name UNUSED, + const char *remote_url UNUSED) +{ + if (accept == ACCEPT_ALL) + return 1; + + BUG("Unhandled 'enum accept_promisor' value '%d'", accept); +} + +static void filter_promisor_remote(struct repository *repo, + struct strvec *accepted, + const char *info) +{ + struct strbuf **remotes; + char *accept_str; + enum accept_promisor accept = ACCEPT_NONE; + + if (!git_config_get_string("promisor.acceptfromserver", &accept_str)) { + if (!accept_str || !*accept_str || !strcasecmp("None", accept_str)) + accept = ACCEPT_NONE; + else if (!strcasecmp("All", accept_str)) + accept = ACCEPT_ALL; + else + warning(_("unknown '%s' value for '%s' config option"), + accept_str, "promisor.acceptfromserver"); + } + + if (accept == ACCEPT_NONE) + return; + + /* Parse remote info received */ + + remotes = strbuf_split_str(info, ';', 0); + + for (size_t i = 0; remotes[i]; i++) { + struct strbuf **elems; + const char *remote_name = NULL; + const char *remote_url = NULL; + char *decoded_url = NULL; + + strbuf_trim_trailing_ch(remotes[i], ';'); + elems = strbuf_split_str(remotes[i]->buf, ',', 0); + + for (size_t j = 0; elems[j]; j++) { + int res; + strbuf_trim_trailing_ch(elems[j], ','); + res = skip_prefix(elems[j]->buf, "name=", &remote_name) || + skip_prefix(elems[j]->buf, "url=", &remote_url); + if (!res) + warning(_("unknown element '%s' from remote info"), + elems[j]->buf); + } + + decoded_url = url_decode(remote_url); + + if (should_accept_remote(accept, remote_name, decoded_url)) + strvec_push(accepted, remote_name); + + strbuf_list_free(elems); + free(decoded_url); + } + + free(accept_str); + strbuf_list_free(remotes); +} + +char *promisor_remote_reply(const char *info) +{ + struct strvec accepted = STRVEC_INIT; + struct strbuf reply = STRBUF_INIT; + + filter_promisor_remote(the_repository, &accepted, info); + + strbuf_addch(&reply, '='); + + for (size_t i = 0; i < accepted.nr; i++) { + if (i != 0) + strbuf_addch(&reply, ';'); + strbuf_addstr(&reply, accepted.v[i]); + } + + strvec_clear(&accepted); + + return strbuf_detach(&reply, NULL); +} + +void mark_promisor_remotes_as_accepted(struct repository *r, const char *remotes) +{ + struct strbuf **accepted_remotes = strbuf_split_str(remotes, ';', 0); + + for (size_t i = 0; accepted_remotes[i]; i++) { + struct promisor_remote *p; + + strbuf_trim_trailing_ch(accepted_remotes[i], ';'); + p = repo_promisor_remote_find(r, accepted_remotes[i]->buf); + if (p) + p->accepted = 1; + else + warning(_("accepted promisor remote '%s' not found"), + accepted_remotes[i]->buf); + } + + strbuf_list_free(accepted_remotes); +} diff --git a/promisor-remote.h b/promisor-remote.h index 88cb599c39..82f060b5af 100644 --- a/promisor-remote.h +++ b/promisor-remote.h @@ -9,11 +9,13 @@ struct object_id; * Promisor remote linked list * * Information in its fields come from remote.XXX config entries or - * from extensions.partialclone. + * from extensions.partialclone, except for 'accepted' which comes + * from protocol v2 capabilities exchange. */ struct promisor_remote { struct promisor_remote *next; char *partial_clone_filter; + unsigned int accepted : 1; const char name[FLEX_ARRAY]; }; @@ -32,4 +34,26 @@ void promisor_remote_get_direct(struct repository *repo, const struct object_id *oids, int oid_nr); +/* + * Append promisor remote info to buf. Useful for a server to + * advertise the promisor remotes it uses. + */ +void promisor_remote_info(struct repository *repo, struct strbuf *buf); + +/* + * Prepare a reply to a "promisor-remote" advertisement from a server. + */ +char *promisor_remote_reply(const char *info); + +/* + * Set the 'accepted' flag for some promisor remotes. Useful when some + * promisor remotes have been accepted by the client. + */ +void mark_promisor_remotes_as_accepted(struct repository *repo, const char *remotes); + +/* + * Has any promisor remote been accepted by the client? + */ +int repo_has_accepted_promisor_remote(struct repository *r); + #endif /* PROMISOR_REMOTE_H */ diff --git a/serve.c b/serve.c index 884cd84ca8..7c5c7c9856 100644 --- a/serve.c +++ b/serve.c @@ -12,6 +12,7 @@ #include "upload-pack.h" #include "bundle-uri.h" #include "trace2.h" +#include "promisor-remote.h" static int advertise_sid = -1; static int advertise_object_info = -1; @@ -31,6 +32,21 @@ static int agent_advertise(struct repository *r UNUSED, return 1; } +static int promisor_remote_advertise(struct repository *r, + struct strbuf *value) +{ + if (value) + promisor_remote_info(r, value); + return 1; +} + +static void promisor_remote_receive(struct repository *r, + const char *remotes) +{ + mark_promisor_remotes_as_accepted(r, remotes); +} + + static int object_format_advertise(struct repository *r, struct strbuf *value) { @@ -157,6 +173,11 @@ static struct protocol_capability capabilities[] = { .advertise = bundle_uri_advertise, .command = bundle_uri_command, }, + { + .name = "promisor-remote", + .advertise = promisor_remote_advertise, + .receive = promisor_remote_receive, + }, }; void protocol_v2_advertise_capabilities(void) diff --git a/t/t5555-http-smart-common.sh b/t/t5555-http-smart-common.sh index 3dcb3340a3..27300a8bf5 100755 --- a/t/t5555-http-smart-common.sh +++ b/t/t5555-http-smart-common.sh @@ -131,6 +131,7 @@ test_expect_success 'git upload-pack --advertise-refs: v2' ' fetch=shallow wait-for-done server-option object-format=$(test_oid algo) + promisor-remote 0000 EOF diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index c48830de8f..c858c43db2 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -22,6 +22,7 @@ test_expect_success 'test capability advertisement' ' object-format=$(test_oid algo) EOF cat >expect.trailer <<-EOF && + promisor-remote 0000 EOF cat expect.base expect.trailer >expect && diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh new file mode 100755 index 0000000000..7e44ad15ce --- /dev/null +++ b/t/t5710-promisor-remote-capability.sh @@ -0,0 +1,124 @@ +#!/bin/sh + +test_description='handling of promisor remote advertisement' + +. ./test-lib.sh + +# Setup the repository with three commits, this way HEAD is always +# available and we can hide commit 1 or 2. +test_expect_success 'setup: create "template" repository' ' + git init template && + test_commit -C template 1 && + test_commit -C template 2 && + test_commit -C template 3 && + test-tool genrandom foo 10240 >template/foo && + git -C template add foo && + git -C template commit -m foo +' + +# A bare repo will act as a server repo with unpacked objects. +test_expect_success 'setup: create bare "server" repository' ' + git clone --bare --no-local template server && + mv server/objects/pack/pack-* . && + packfile=$(ls pack-*.pack) && + git -C server unpack-objects --strict <"$packfile" +' + +check_missing_objects () { + git -C "$1" rev-list --objects --all --missing=print > all.txt && + perl -ne 'print if s/^[?]//' all.txt >missing.txt && + test_line_count = "$2" missing.txt && + test "$3" = "$(cat missing.txt)" +} + +initialize_server () { + # Repack everything first + git -C server -c repack.writebitmaps=false repack -a -d && + + # Remove promisor file in case they exist, useful when reinitializing + rm -rf server/objects/pack/*.promisor && + + # Repack without the largest object and create a promisor pack on server + git -C server -c repack.writebitmaps=false repack -a -d \ + --filter=blob:limit=5k --filter-to="$(pwd)" && + promisor_file=$(ls server/objects/pack/*.pack | sed "s/\.pack/.promisor/") && + touch "$promisor_file" && + + # Check that only one object is missing on the server + check_missing_objects server 1 "$oid" +} + +test_expect_success "setup for testing promisor remote advertisement" ' + # Create another bare repo called "server2" + git init --bare server2 && + + # Copy the largest object from server to server2 + obj="HEAD:foo" && + oid="$(git -C server rev-parse $obj)" && + oid_path="$(test_oid_to_path $oid)" && + path="server/objects/$oid_path" && + path2="server2/objects/$oid_path" && + mkdir -p $(dirname "$path2") && + cp "$path" "$path2" && + + initialize_server && + + # Configure server2 as promisor remote for server + git -C server remote add server2 "file://$(pwd)/server2" && + git -C server config remote.server2.promisor true && + + git -C server2 config uploadpack.allowFilter true && + git -C server2 config uploadpack.allowAnySHA1InWant true && + git -C server config uploadpack.allowFilter true && + git -C server config uploadpack.allowAnySHA1InWant true +' + +test_expect_success "fetch with promisor.advertise set to 'true'" ' + git -C server config promisor.advertise true && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ + -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.server2.url="file://$(pwd)/server2" \ + -c promisor.acceptfromserver=All \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + +test_expect_success "fetch with promisor.advertise set to 'false'" ' + git -C server config promisor.advertise false && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ + -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.server2.url="file://$(pwd)/server2" \ + -c promisor.acceptfromserver=All \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + + # Check that the largest object is not missing on the server + check_missing_objects server 0 "" && + + # Reinitialize server so that the largest object is missing again + initialize_server +' + +test_expect_success "fetch with promisor.acceptfromserver set to 'None'" ' + git -C server config promisor.advertise true && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ + -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.server2.url="file://$(pwd)/server2" \ + -c promisor.acceptfromserver=None \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + + # Check that the largest object is not missing on the server + check_missing_objects server 0 "" +' + +test_done diff --git a/upload-pack.c b/upload-pack.c index 0052c6a4dc..0cff76c845 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -31,6 +31,7 @@ #include "write-or-die.h" #include "json-writer.h" #include "strmap.h" +#include "promisor-remote.h" /* Remember to update object flag allocation in object.h */ #define THEY_HAVE (1u << 11) @@ -317,6 +318,8 @@ static void create_pack_file(struct upload_pack_data *pack_data, strvec_push(&pack_objects.args, "--delta-base-offset"); if (pack_data->use_include_tag) strvec_push(&pack_objects.args, "--include-tag"); + if (repo_has_accepted_promisor_remote(the_repository)) + strvec_push(&pack_objects.args, "--missing=allow-promisor"); if (pack_data->filter_options.choice) { const char *spec = expand_list_objects_filter_spec(&pack_data->filter_options); From patchwork Wed Jul 31 13:40:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Couder X-Patchwork-Id: 13748816 Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 372281B372C for ; Wed, 31 Jul 2024 13:40:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433246; cv=none; b=VOgTFX+oHQ/rZTsiMkHuabok6NyTXNy/iPiq98U7ZPX1cn1XYlRgkGWL0N2HUuA2MNvcXYSJt/l0HmseYNJ+UGcBaU/j8syon30p6j7QrqsBDXZVbUUhpJC7ONnaE5HvLLoXsZ+UbEL+Eq++uRbuF2bQa+8jXY/5ka5pUCOXYUA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722433246; c=relaxed/simple; bh=IV3LyB3H+qRJhDo6yYig/Uh/ITF+82iHxAK4K7d40es=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=uWTlWPaw2E7MITJ8lFOXexnD+m4+6gqjd4ou82ndp5Nm06wMEiBDuwbSC54vG0Exj2wfsb0ONUO9Na7EP+qETxmCPw8/9q4Dpqwt6uRTdA3GdCM1LOuDL9Bqat6taDA0uFCe8VLCIG7FcPpmPDLeh8LG+zv9jn6Qcwl1NVg072k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Iti2xsKE; arc=none smtp.client-ip=209.85.167.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Iti2xsKE" Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-530ad969360so1789610e87.0 for ; Wed, 31 Jul 2024 06:40:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722433242; x=1723038042; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=X4ZthYY/Jj9GwL8xZn+HzFCMcpMgA7bNZxBgJ22wksA=; b=Iti2xsKEFHE8WR2l7kjKvlviFSSJcacGS6o3IBTkcLiSnSfb9vHMhG2jw7MLeeyS6R 12+MW3ioYXqEMCIPjBcM6K6m6PWpGaHnz/caPS7Inqvr/fcOrVhEQSjFSaWT0s1s9YGN rrkB3UBwivXOHF4+yOplBFsQ2YYiHxwrkJqI6V0bpW4J75aLfNSZRSoYe9bLjf1vBp/i NvpKDVo8SFabF4ggyqAFIqvgeZaxceAT94+uzuEqQEJvRejb+BcP6GZYWXtIw6tylWsy rVycnH1DQnDEt9cUap8cBktqYkstM8XJSPv1DPMqxdcOTH/sYG3xrwuXye1fzMlHbr+f hyzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722433242; x=1723038042; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=X4ZthYY/Jj9GwL8xZn+HzFCMcpMgA7bNZxBgJ22wksA=; b=d9actuLGJvR59W3kTqFYaEaCYOsH6vcW3g9D0qUm3LMZuEgNMCot8LIR5rg9xMtpA0 RmBP+VOrzATMNUTlJTQzI1jBZ6NXOTVZBMijiNZdiDqcOBkRn6mFv1yHKqtrWYs5NUOL HyMfq4hSWy7xkeVN4kIgSLYt7UBXXu325GlNV8JMHQhAwKXUinCnQ5l4hDUfhMohyjbX 3zJlxh1ZHD+3SFX3En1t66xB01UA3NlXWIrL/N6bNoIwm/rw+PGI/EDx3i4r1BhNg3OB fW3HdUTUCE093G8u5zIBBFUK1Sw5GROIFM+ldn3gijm5h6lWHNwFcFyrY+/RVKTCUprv Qxhw== X-Gm-Message-State: AOJu0YwInE0r7BTAyq1jita4Q1Bpi8pnh3VK3sxe/UdI6kxrQTp+HJM7 cfvSmNxPFnpvO0arrOoxo4z5rZwmI9xvMa1uluf/B3ZAxGGkPGMFdmklmg== X-Google-Smtp-Source: AGHT+IG8rcLwW6mTVQcJx3ET7HES/xeXT79dqvGZFjf7BksL+zvG0+qmuumQDprqwPQtNDg3Z6rp6A== X-Received: by 2002:ac2:4d9c:0:b0:52c:86d7:fa62 with SMTP id 2adb3069b0e04-5309b272327mr8940670e87.23.1722433241164; Wed, 31 Jul 2024 06:40:41 -0700 (PDT) Received: from christian-Precision-5550.. (176-138-135-207.abo.bbox.fr. [176.138.135.207]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5ac63c50fdesm8669641a12.56.2024.07.31.06.40.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jul 2024 06:40:40 -0700 (PDT) From: Christian Couder To: git@vger.kernel.org Cc: Junio C Hamano , John Cai , Patrick Steinhardt , Christian Couder , Christian Couder Subject: [PATCH 4/4] promisor-remote: check advertised name or URL Date: Wed, 31 Jul 2024 15:40:14 +0200 Message-ID: <20240731134014.2299361-5-christian.couder@gmail.com> X-Mailer: git-send-email 2.46.0.4.gbcb884ee16 In-Reply-To: <20240731134014.2299361-1-christian.couder@gmail.com> References: <20240731134014.2299361-1-christian.couder@gmail.com> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 A previous commit introduced a "promisor.acceptFromServer" configuration variable with only "None" or "All" as valid values. Let's introduce "KnownName" and "KnownUrl" as valid values for this configuration option to give more choice to a client about which promisor remotes it might accept among those that the server advertised. In case of "KnownName", the client will accept promisor remotes which are already configured on the client and have the same name as those advertised by the client. In case of "KnownUrl", the client will accept promisor remotes which have both the same name and the same URL configured on the client as the name and URL advertised by the server. Signed-off-by: Christian Couder --- Documentation/config/promisor.txt | 11 +++-- promisor-remote.c | 54 +++++++++++++++++++-- t/t5710-promisor-remote-capability.sh | 68 +++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 7 deletions(-) diff --git a/Documentation/config/promisor.txt b/Documentation/config/promisor.txt index e3939d83a9..fadf593621 100644 --- a/Documentation/config/promisor.txt +++ b/Documentation/config/promisor.txt @@ -11,6 +11,11 @@ promisor.advertise:: promisor.acceptFromServer:: If set to "all", a client will accept all the promisor remotes a server might advertise using the "promisor-remote" - capability, see linkgit:gitprotocol-v2[5]. Default is "none", - which means no promisor remote advertised by a server will be - accepted. + capability, see linkgit:gitprotocol-v2[5]. If set to + "knownName" the client will accept promisor remotes which are + already configured on the client and have the same name as + those advertised by the client. If set to "knownUrl", the + client will accept promisor remotes which have both the same + name and the same URL configured on the client as the name and + URL advertised by the server. Default is "none", which means + no promisor remote advertised by a server will be accepted. diff --git a/promisor-remote.c b/promisor-remote.c index d347f4d9b5..0ff26b835e 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -362,19 +362,54 @@ void promisor_remote_info(struct repository *repo, struct strbuf *buf) strvec_clear(&urls); } +/* + * Find first index of 'vec' where there is 'val'. 'val' is compared + * case insensively to the strings in 'vec'. If not found 'vec->nr' is + * returned. + */ +static size_t strvec_find_index(struct strvec *vec, const char *val) +{ + for (size_t i = 0; i < vec->nr; i++) + if (!strcasecmp(vec->v[i], val)) + return i; + return vec->nr; +} + enum accept_promisor { ACCEPT_NONE = 0, + ACCEPT_KNOWN_URL, + ACCEPT_KNOWN_NAME, ACCEPT_ALL }; static int should_accept_remote(enum accept_promisor accept, - const char *remote_name UNUSED, - const char *remote_url UNUSED) + const char *remote_name, const char *remote_url, + struct strvec *names, struct strvec *urls) { + size_t i; + if (accept == ACCEPT_ALL) return 1; - BUG("Unhandled 'enum accept_promisor' value '%d'", accept); + i = strvec_find_index(names, remote_name); + + if (i >= names->nr) + /* We don't know about that remote */ + return 0; + + if (accept == ACCEPT_KNOWN_NAME) + return 1; + + if (accept != ACCEPT_KNOWN_URL) + BUG("Unhandled 'enum accept_promisor' value '%d'", accept); + + if (!strcasecmp(urls->v[i], remote_url)) + return 1; + + warning(_("known remote named '%s' but with url '%s' instead of '%s'"), + remote_name, urls->v[i], remote_url); + + return 0; } static void filter_promisor_remote(struct repository *repo, @@ -384,10 +419,16 @@ static void filter_promisor_remote(struct repository *repo, struct strbuf **remotes; char *accept_str; enum accept_promisor accept = ACCEPT_NONE; + struct strvec names = STRVEC_INIT; + struct strvec urls = STRVEC_INIT; if (!git_config_get_string("promisor.acceptfromserver", &accept_str)) { if (!accept_str || !*accept_str || !strcasecmp("None", accept_str)) accept = ACCEPT_NONE; + else if (!strcasecmp("KnownUrl", accept_str)) + accept = ACCEPT_KNOWN_URL; + else if (!strcasecmp("KnownName", accept_str)) + accept = ACCEPT_KNOWN_NAME; else if (!strcasecmp("All", accept_str)) accept = ACCEPT_ALL; else @@ -398,6 +439,9 @@ static void filter_promisor_remote(struct repository *repo, if (accept == ACCEPT_NONE) return; + if (accept != ACCEPT_ALL) + promisor_info_vecs(repo, &names, &urls); + /* Parse remote info received */ remotes = strbuf_split_str(info, ';', 0); @@ -423,7 +467,7 @@ static void filter_promisor_remote(struct repository *repo, decoded_url = url_decode(remote_url); - if (should_accept_remote(accept, remote_name, decoded_url)) + if (should_accept_remote(accept, remote_name, decoded_url, &names, &urls)) strvec_push(accepted, remote_name); strbuf_list_free(elems); @@ -431,6 +475,8 @@ static void filter_promisor_remote(struct repository *repo, } free(accept_str); + strvec_clear(&names); + strvec_clear(&urls); strbuf_list_free(remotes); } diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh index 7e44ad15ce..c2c83a5914 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -117,6 +117,74 @@ test_expect_success "fetch with promisor.acceptfromserver set to 'None'" ' --no-local --filter="blob:limit=5k" server client && test_when_finished "rm -rf client" && + # Check that the largest object is not missing on the server + check_missing_objects server 0 "" && + + # Reinitialize server so that the largest object is missing again + initialize_server +' + +test_expect_success "fetch with promisor.acceptfromserver set to 'KnownName'" ' + git -C server config promisor.advertise true && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ + -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.server2.url="file://$(pwd)/server2" \ + -c promisor.acceptfromserver=KnownName \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + +test_expect_success "fetch with 'KnownName' and different remote names" ' + git -C server config promisor.advertise true && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.serverTwo.promisor=true \ + -c remote.serverTwo.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.serverTwo.url="file://$(pwd)/server2" \ + -c promisor.acceptfromserver=KnownName \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + + # Check that the largest object is not missing on the server + check_missing_objects server 0 "" && + + # Reinitialize server so that the largest object is missing again + initialize_server +' + +test_expect_success "fetch with promisor.acceptfromserver set to 'KnownUrl'" ' + git -C server config promisor.advertise true && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ + -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.server2.url="file://$(pwd)/server2" \ + -c promisor.acceptfromserver=KnownUrl \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + +test_expect_success "fetch with 'KnownUrl' and different remote urls" ' + ln -s server2 serverTwo && + + git -C server config promisor.advertise true && + + # Clone from server to create a client + GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ + -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ + -c remote.server2.url="file://$(pwd)/serverTwo" \ + -c promisor.acceptfromserver=KnownUrl \ + --no-local --filter="blob:limit=5k" server client && + test_when_finished "rm -rf client" && + # Check that the largest object is not missing on the server check_missing_objects server 0 "" '