* Re: [PATCH v6 1/6] submodules: add helper functions to determine presence of submodules
From: Brandon Williams @ 2016-12-01 19:16 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, sbeller, jonathantanmy
In-Reply-To: <20161201190925.xi2z7vauxyf3yxyc@sigill.intra.peff.net>
On 12/01, Jeff King wrote:
> On Thu, Dec 01, 2016 at 10:46:23AM -0800, Junio C Hamano wrote:
>
> > > mkpath() is generally an unsafe function because it uses a static
> > > buffer, but it's handy and safe for handing values to syscalls like
> > > this.
> >
> > I think your "unsafe" is not about thread-safety but about "the
> > caller cannot rely on returned value staying valid for long haul".
> > If this change since v5 is about thread-safety, I am not sure if it
> > is safe to use mkpath here.
>
> Oh, good point. I meant "staying valid", but somehow totally forgot that
> we cared about thread reentrancy here. As if I hadn't just spent an hour
> debugging a thread problem.
>
> My suggestion is clearly nonsense.
>
> > I am a bit wary of making the check too sketchy like this, but this
> > is not about determining if a random "path" that has ".git" in a
> > superproject working tree is a submodule or not (that information
> > primarily comes from the superproject index), so I tend to agree
> > with the patch that it is sufficient to check presence of ".git"
> > alone.
>
> The real danger is that it is a different check than the child process
> is going to use, so they may disagree (see the almost-infinite-loop
> discussion elsewhere).
>
> It feels quite hacky, but checking:
>
> if (is_git_directory(suspect))
> return 1; /* actual git dir */
> if (!stat(suspect, &st) && S_ISREG(st.st_mode))
> return 1; /* gitfile; may or may not be valid */
> return 0;
>
> is a little more robust, because the child process will happily skip a
> non-repo ".git" and keep walking back up to the superproject. Whereas if
> it sees any ".git" file, even if it is bogus, it will barf then and
> there.
>
> I'm actually not sure if that latter behavior is a bug or not. I don't
> think it was really planned out, and it obviously is inconsistent with
> the other repo-discovery cases. But it is a convenient side effect for
> submodules, and I doubt anybody is bothered by it in practice.
>
> -Peff
I think this more robust check is probably a good idea, that way we
don't step into a submodule with a .git directory that isn't really a
.git dir.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v6 1/6] submodules: add helper functions to determine presence of submodules
From: Jeff King @ 2016-12-01 19:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, git, sbeller, jonathantanmy
In-Reply-To: <xmqqwpfja3nk.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 01, 2016 at 10:46:23AM -0800, Junio C Hamano wrote:
> > mkpath() is generally an unsafe function because it uses a static
> > buffer, but it's handy and safe for handing values to syscalls like
> > this.
>
> I think your "unsafe" is not about thread-safety but about "the
> caller cannot rely on returned value staying valid for long haul".
> If this change since v5 is about thread-safety, I am not sure if it
> is safe to use mkpath here.
Oh, good point. I meant "staying valid", but somehow totally forgot that
we cared about thread reentrancy here. As if I hadn't just spent an hour
debugging a thread problem.
My suggestion is clearly nonsense.
> I am a bit wary of making the check too sketchy like this, but this
> is not about determining if a random "path" that has ".git" in a
> superproject working tree is a submodule or not (that information
> primarily comes from the superproject index), so I tend to agree
> with the patch that it is sufficient to check presence of ".git"
> alone.
The real danger is that it is a different check than the child process
is going to use, so they may disagree (see the almost-infinite-loop
discussion elsewhere).
It feels quite hacky, but checking:
if (is_git_directory(suspect))
return 1; /* actual git dir */
if (!stat(suspect, &st) && S_ISREG(st.st_mode))
return 1; /* gitfile; may or may not be valid */
return 0;
is a little more robust, because the child process will happily skip a
non-repo ".git" and keep walking back up to the superproject. Whereas if
it sees any ".git" file, even if it is bogus, it will barf then and
there.
I'm actually not sure if that latter behavior is a bug or not. I don't
think it was really planned out, and it obviously is inconsistent with
the other repo-discovery cases. But it is a convenient side effect for
submodules, and I doubt anybody is bothered by it in practice.
-Peff
^ permalink raw reply
* Re: [PATCH v6 0/6] recursively grep across submodules
From: Jeff King @ 2016-12-01 19:03 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, jonathantanmy, gitster
In-Reply-To: <20161201174547.GA51406@google.com>
On Thu, Dec 01, 2016 at 09:45:47AM -0800, Brandon Williams wrote:
> Yeah I was trying to think through these scenarios myself last night.
> And like you found it seemed alright to let the child process deal with
> the .git file/dir as long as once actually exists at that path. If one
> didn't then there would be the possibility that we ended up back at the
> superproject, which would result in an infinite loop. And yeah if the
> .git file doesn't resolve to anything sensible then the user probably
> mangled their repository somehow anyways.
I hadn't considered the infinite loop. I thought the worst case is that
we might just generate bogus results by going back to the superproject.
But of course there is nothing to stop it from just recursing again.
However, it looks like there is a circuit-breaker; we end up back in the
superproject, but inside a subdirectory, which causes --super-prefix to
complain.
You can test it with just:
rm submodule/.git
mkdir submodule/.git
which says:
fatal: can't use --super-prefix from a subdirectory
fatal: process for submodule 'foo' failed with exit code: 128
It might be worth including a test to make sure that behavior remains.
I think it's more of an emergent behavior than something planned. :)
-Peff
^ permalink raw reply
* Re: [PATCH v6 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Jeff King @ 2016-12-01 18:52 UTC (permalink / raw)
To: Brandon Williams; +Cc: Johannes Sixt, git, sbeller, jonathantanmy, gitster
In-Reply-To: <20161201175107.GB51406@google.com>
On Thu, Dec 01, 2016 at 09:51:07AM -0800, Brandon Williams wrote:
> On 12/01, Johannes Sixt wrote:
> > Am 01.12.2016 um 02:28 schrieb Brandon Williams:
> > >+ git init "su:b" &&
> >
> > Don't do that. Colons in file names won't work on Windows.
> >
> > -- Hannes
> >
>
> This test is needed to see if the code still works with filenames that
> contain colons. Is there a way to mark the test to not run on windows?
Junio suggested !MINGW, which seems sensible. Earlier I mentioned doing
the whole thing in-index, but I think that might get tricky because we
try to find the submodule as ".git/modules/<path>". So it probably isn't
worth the trouble.
-Peff
^ permalink raw reply
* Re: Re* git pull --rebase should use fast forward merge if possible
From: Stefan Beller @ 2016-12-01 18:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, neuling
In-Reply-To: <xmqqoa0va3fw.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 1, 2016 at 10:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Thu, Dec 1, 2016 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> +test_expect_success '--rebase fast forward' '
>>> + git reset --hard before-rebase &&
>>> + git checkout -b ff &&
>>> + echo another modification >file &&
>>> + git commit -m third file &&
>>> +
>>> + git checkout to-rebase &&
>>> + git pull --rebase . ff &&
>>> + test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" &&
>>> +
>>> + # The above only validates the result. Did we actually bypass rebase?
>>
>> Good catch for the test, but I think we can make the sed regexp simpler, as we
>> can leave out the second "[0-9a-f]"? (git reflog |sed
>> "s/^[0-9a-f]*/OBJID/" works here)
>
> This mimics the existing tests around there for consistency.
> Simplifying or cleaning of this test script as a whole is outside
> the scope.
Ok, in that case the patch looks fine.
^ permalink raw reply
* Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Jeff King @ 2016-12-01 18:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, Johannes Schindelin, GIT Mailing-list
In-Reply-To: <xmqq60n3bjel.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 01, 2016 at 10:20:50AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I don't have a preference on which direction we go, but yes, right now
> > we are in an awkward middle ground. We should do one of:
> >
> > 1. Drop -Wno-format-zero-length from DEVELOPER_CFLAGS and make sure
> > future patches to do not violate it.
> >
> > 2. Declare warning("") as OK.
> >
> > I still think the warning is silly, but (1) has value in that it
> > produces the least surprise and annoyance to various people building
> > Git.
>
> What is most awkward is that "make", with no customization out of
> box, suggests to use -Wall in CFLAGS and triggers the misguided
> warning, and DEVELOPER_CFLAGS, partly because it adds -Werror to
> turn warnings (including this misguided one) into errors, disables
> it with -Wno-format-zero-length. As a result:
>
> - Casual builders that follow our suggestion get warnings.
>
> - Developers do not notice their new code may make the "casual
> builder" experience worse.
>
> That is totally backwards. That is probably what you meant by "the
> least surprise and annoyance"?
Yes, exactly. :)
The surprise and annoyance for (1) is that people who get caught up by
the warning while writing new patches say "this warning is stupid, why
don't we disable it". But that is a much smaller number of people to be
annoyed.
> I also still think that any_printf_like_function("%s", "") looks
> silly. I know that we've already started moving in that direction
> and we stopped at a place we do not want to be in, but perhaps it
> was a mistake to move in that direction in the first place. I am
> tempted to say we should do something like the attached, but that
> may not fly well, as I suspect that -Wno-format-zero-length may be a
> lot more exotic than the -Wall compiler option.
Yeah, I think portability concerns are what caused us not to go down
that road in the first place. But I think it's also a mistake to put too
much into CFLAGS, because somebody who wants to override one flag has to
repeat the rest. So, e.g., right now I might do:
make CFLAGS="-O3 -Wall"
to bump the optimization level. But if our codebase relies on
-Wno-format-zero-length being there, then I have to know to put it in
there, too.
You can work around that with an extra make variable, but that also
makes it harder to disable if your compiler doesn't like it.
> An obvious second
> best option would be to drop -Wall from the "everybody" CFLAGS and
> move it to DEVELOPER_CFLAGS instead.
Yeah, though that doesn't help the example above.
As ugly as warning("%s", "") is, I think it may be the thing that annoys
the smallest number of people.
-Peff
^ permalink raw reply
* Re: Re* git pull --rebase should use fast forward merge if possible
From: Junio C Hamano @ 2016-12-01 18:50 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, neuling
In-Reply-To: <CAGZ79kZSEan5uXCUn4iVCWEc9zohMSr+UDyHDyQUHz84H=tR8w@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Thu, Dec 1, 2016 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> +test_expect_success '--rebase fast forward' '
>> + git reset --hard before-rebase &&
>> + git checkout -b ff &&
>> + echo another modification >file &&
>> + git commit -m third file &&
>> +
>> + git checkout to-rebase &&
>> + git pull --rebase . ff &&
>> + test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" &&
>> +
>> + # The above only validates the result. Did we actually bypass rebase?
>
> Good catch for the test, but I think we can make the sed regexp simpler, as we
> can leave out the second "[0-9a-f]"? (git reflog |sed
> "s/^[0-9a-f]*/OBJID/" works here)
This mimics the existing tests around there for consistency.
Simplifying or cleaning of this test script as a whole is outside
the scope.
^ permalink raw reply
* Re: [PATCH v6 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Junio C Hamano @ 2016-12-01 18:49 UTC (permalink / raw)
To: Brandon Williams; +Cc: Johannes Sixt, git, peff, sbeller, jonathantanmy
In-Reply-To: <20161201175107.GB51406@google.com>
Brandon Williams <bmwill@google.com> writes:
> On 12/01, Johannes Sixt wrote:
>> Am 01.12.2016 um 02:28 schrieb Brandon Williams:
>> >+ git init "su:b" &&
>>
>> Don't do that. Colons in file names won't work on Windows.
>>
>> -- Hannes
>>
>
> This test is needed to see if the code still works with filenames that
> contain colons. Is there a way to mark the test to not run on windows?
Something like:
test_expect_success !MINGW 'a test' '
git init s:u:b
'
^ permalink raw reply
* Re: [PATCH v6 1/6] submodules: add helper functions to determine presence of submodules
From: Junio C Hamano @ 2016-12-01 18:46 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git, sbeller, jonathantanmy
In-Reply-To: <20161201042926.mr2qdta7hviizcya@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Nov 30, 2016 at 05:28:29PM -0800, Brandon Williams wrote:
>
>> +/*
>> + * Determine if a submodule has been populated at a given 'path'
>> + */
>> +int is_submodule_populated(const char *path)
>> +{
>> + int ret = 0;
>> + struct stat st;
>> + char *gitdir = xstrfmt("%s/.git", path);
>> +
>> + if (!stat(gitdir, &st))
>> + ret = 1;
>> +
>> + free(gitdir);
>> + return ret;
>> +}
>
> I don't know if it's worth changing or not, but this could be a bit
> shorter:
>
> int is_submodule_populated(const char *path)
> {
> return !access(mkpath("%s/.git", path), F_OK);
> }
>
> There is a file_exists() helper, but it uses lstat(), which I think you
> don't want (because you'd prefer to bail on a broken .git symlink). But
> access(F_OK) does what you want, I think.
>
> mkpath() is generally an unsafe function because it uses a static
> buffer, but it's handy and safe for handing values to syscalls like
> this.
I think your "unsafe" is not about thread-safety but about "the
caller cannot rely on returned value staying valid for long haul".
If this change since v5 is about thread-safety, I am not sure if it
is safe to use mkpath here.
I am a bit wary of making the check too sketchy like this, but this
is not about determining if a random "path" that has ".git" in a
superproject working tree is a submodule or not (that information
primarily comes from the superproject index), so I tend to agree
with the patch that it is sufficient to check presence of ".git"
alone.
^ permalink raw reply
* Re: [PATCH v6 1/6] submodules: add helper functions to determine presence of submodules
From: Stefan Beller @ 2016-12-01 18:31 UTC (permalink / raw)
To: Jeff King
Cc: Brandon Williams, git@vger.kernel.org, Jonathan Tan,
Junio C Hamano
In-Reply-To: <20161201042926.mr2qdta7hviizcya@sigill.intra.peff.net>
On Wed, Nov 30, 2016 at 8:29 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 30, 2016 at 05:28:29PM -0800, Brandon Williams wrote:
>
>> +/*
>> + * Determine if a submodule has been populated at a given 'path'
>> + */
>> +int is_submodule_populated(const char *path)
>> +{
>> + int ret = 0;
>> + struct stat st;
>> + char *gitdir = xstrfmt("%s/.git", path);
>> +
>> + if (!stat(gitdir, &st))
>> + ret = 1;
>> +
>> + free(gitdir);
>> + return ret;
>> +}
>
> I don't know if it's worth changing or not, but this could be a bit
> shorter:
>
> int is_submodule_populated(const char *path)
> {
> return !access(mkpath("%s/.git", path), F_OK);
> }
>
> There is a file_exists() helper, but it uses lstat(), which I think you
> don't want (because you'd prefer to bail on a broken .git symlink). But
> access(F_OK) does what you want, I think.
>
> mkpath() is generally an unsafe function because it uses a static
> buffer, but it's handy and safe for handing values to syscalls like
> this.
>
> I say "I don't know if it's worth it" because what you've written is
> fine, and while more lines, it's fairly obvious and safe.
OK, chiming in here as well. :)
I plan on making use of the is_submodule_populated method in
the checkout --recurse-submodules series, and for that I am
undecided whether a cheap stat is the right approach or if we want
to have the result of resolve_gitdir as that fails in weird corner cases.
Anyway, I'd propose to change the name when going with either the
code as is or what Jeff proposes to be one of
is_submodule_populated_cheaply
is_submodule_populated_with_no_sanity_check
is_submodule_dot_git_present
have_submodule_dot_git
I think I'd prefer the last one as that describes what the function
actually does in a concise way?
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v2 1/1] convert: git cherry-pick -Xrenormalize did not work
From: Junio C Hamano @ 2016-12-01 18:27 UTC (permalink / raw)
To: tboegi; +Cc: git, eevee.reply
In-Reply-To: <20161130170232.19685-1-tboegi@web.de>
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> Working with a repo that used to be all CRLF. At some point it
> was changed to all LF, with `text=auto` in .gitattributes.
> Trying to cherry-pick a commit from before the switchover fails:
>
> $ git cherry-pick -Xrenormalize <commit>
> fatal: CRLF would be replaced by LF in [path]
>
> Commit 65237284 "unify the "auto" handling of CRLF" introduced
> a regression:
>
> Whenever crlf_action is CRLF_TEXT_XXX and not CRLF_AUTO_XXX,
> SAFE_CRLF_RENORMALIZE was feed into check_safe_crlf().
> This is wrong because here everything else than SAFE_CRLF_WARN is
> treated as SAFE_CRLF_FAIL.
>
> Call check_safe_crlf() only if checksafe is SAFE_CRLF_WARN or SAFE_CRLF_FAIL.
>
> Reported-by: Eevee (Lexy Munroe) <eevee@veekun.com>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
I think the description and the code match with each other much
better and the resulting code explains what is going on more
clearly. Thanks---will queue.
> convert.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index be91358..f8e4dfe 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -281,13 +281,13 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> /*
> * If the file in the index has any CR in it, do not convert.
> * This is the new safer autocrlf handling.
> + - unless we want to renormalize in a merge or cherry-pick
> */
> - if (checksafe == SAFE_CRLF_RENORMALIZE)
> - checksafe = SAFE_CRLF_FALSE;
> - else if (has_cr_in_index(path))
> + if ((checksafe != SAFE_CRLF_RENORMALIZE) && has_cr_in_index(path))
> convert_crlf_into_lf = 0;
> }
> - if (checksafe && len) {
> + if ((checksafe == SAFE_CRLF_WARN ||
> + (checksafe == SAFE_CRLF_FAIL)) && len) {
> struct text_stat new_stats;
> memcpy(&new_stats, &stats, sizeof(new_stats));
> /* simulate "git add" */
^ permalink raw reply
* Re: Re* git pull --rebase should use fast forward merge if possible
From: Stefan Beller @ 2016-12-01 18:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, neuling
In-Reply-To: <xmqqa8cfbkeq.fsf_-_@gitster.mtv.corp.google.com>
On Thu, Dec 1, 2016 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
> +test_expect_success '--rebase fast forward' '
> + git reset --hard before-rebase &&
> + git checkout -b ff &&
> + echo another modification >file &&
> + git commit -m third file &&
> +
> + git checkout to-rebase &&
> + git pull --rebase . ff &&
> + test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" &&
> +
> + # The above only validates the result. Did we actually bypass rebase?
Good catch for the test, but I think we can make the sed regexp simpler, as we
can leave out the second "[0-9a-f]"? (git reflog |sed
"s/^[0-9a-f]*/OBJID/" works here)
The implication of that we'd also match if there is no object id at
all at the beginning,
which sounds fine.
I shortly debated the idea to just cut off anything before the first
space and then expect
"HEAD@{0}: pull --rebase . ff: Fast-forward" only, but why cut off
what we produce
in the first place?
git reflog --format="%s" -1 >actual &&
echo "pull --rebase . ff: Fast-forward" >expect &&
test_cmp expect actual
maybe?
^ permalink raw reply
* Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Junio C Hamano @ 2016-12-01 18:20 UTC (permalink / raw)
To: Jeff King; +Cc: Ramsay Jones, Johannes Schindelin, GIT Mailing-list
In-Reply-To: <20161201040234.3rnuttitneweedn5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I don't have a preference on which direction we go, but yes, right now
> we are in an awkward middle ground. We should do one of:
>
> 1. Drop -Wno-format-zero-length from DEVELOPER_CFLAGS and make sure
> future patches to do not violate it.
>
> 2. Declare warning("") as OK.
>
> I still think the warning is silly, but (1) has value in that it
> produces the least surprise and annoyance to various people building
> Git.
What is most awkward is that "make", with no customization out of
box, suggests to use -Wall in CFLAGS and triggers the misguided
warning, and DEVELOPER_CFLAGS, partly because it adds -Werror to
turn warnings (including this misguided one) into errors, disables
it with -Wno-format-zero-length. As a result:
- Casual builders that follow our suggestion get warnings.
- Developers do not notice their new code may make the "casual
builder" experience worse.
That is totally backwards. That is probably what you meant by "the
least surprise and annoyance"?
I also still think that any_printf_like_function("%s", "") looks
silly. I know that we've already started moving in that direction
and we stopped at a place we do not want to be in, but perhaps it
was a mistake to move in that direction in the first place. I am
tempted to say we should do something like the attached, but that
may not fly well, as I suspect that -Wno-format-zero-length may be a
lot more exotic than the -Wall compiler option. An obvious second
best option would be to drop -Wall from the "everybody" CFLAGS and
move it to DEVELOPER_CFLAGS instead.
diff --git a/Makefile b/Makefile
index a379738db2..137c10e257 100644
--- a/Makefile
+++ b/Makefile
@@ -391,10 +391,9 @@ GIT-VERSION-FILE: FORCE
# CFLAGS and LDFLAGS are for the users to override from the command line.
-CFLAGS = -g -O2 -Wall
+CFLAGS = -g -O2 -Wall -Wno-format-zero-length
DEVELOPER_CFLAGS = -Werror \
-Wdeclaration-after-statement \
- -Wno-format-zero-length \
-Wold-style-definition \
-Woverflow \
-Wpointer-arith \
^ permalink raw reply related
* Re: bw/transport-protocol-policy
From: Brandon Williams @ 2016-12-01 18:14 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161201083005.dui572o4jxsqacas@sigill.intra.peff.net>
On 12/01, Jeff King wrote:
> On Mon, Nov 28, 2016 at 04:15:08PM -0800, Junio C Hamano wrote:
>
> > * bw/transport-protocol-policy (2016-11-09) 2 commits
> > (merged to 'next' on 2016-11-16 at 1391d3eeed)
> > + transport: add protocol policy config option
> > + lib-proto-disable: variable name fix
> >
> > Finer-grained control of what protocols are allowed for transports
> > during clone/fetch/push have been enabled via a new configuration
> > mechanism.
> >
> > Will cook in 'next'.
>
> I was looking at the way the http code feeds protocol restrictions to
> CURLOPT_REDIR_PROTOCOLS, and I think this topic is missing two elements:
>
> 1. The new policy config lets you say "only allow this protocol when
> the user specifies it". But when http.c calls is_transport_allowed(),
> the latter has no idea that we are asking it about potential
> redirects (which obviously do _not_ come from the user), and would
> erroneously allow them.
>
> I think this needs fixed before the topic is merged. It's not a
> regression, as it only comes into play if you use the new policy
> config. But it is a minor security hole in the new feature.
I agree and it should be an easy fix. We can just add a parameter like
so:
diff --git a/transport.c b/transport.c
index 2c0ec76..d38d50f 100644
--- a/transport.c
+++ b/transport.c
@@ -723,7 +723,7 @@ static enum protocol_allow_config get_protocol_config(const char *type)
return PROTOCOL_ALLOW_USER_ONLY;
}
-int is_transport_allowed(const char *type)
+int is_transport_allowed(const char *type, int redirect)
{
const struct string_list *whitelist = protocol_whitelist();
if (whitelist)
@@ -735,7 +735,7 @@ int is_transport_allowed(const char *type)
case PROTOCOL_ALLOW_NEVER:
return 0;
case PROTOCOL_ALLOW_USER_ONLY:
- return git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
+ return git_env_bool("GIT_PROTOCOL_FROM_USER", !redirect);
}
die("BUG: invalid protocol_allow_config type");
That way the libcurl code can say it is asking if it is ok to redirect
to that protocol.
>
> 2. If your curl is too old to support CURLOPT_REDIR_PROTOCOLS, we will
> warn if there is a protocol whitelist in effect. But that check
> only covers the environment whitelist, and we do not warn if you
> restrict other protocols.
>
> I actually think this should probably just warn indiscriminately.
> Even without a Git protocol whitelist specified, the code serves to
> prevent curl from redirecting to bizarre protocols like smtp. The
> affected curl versions are from 2009 and prior, so I kind of doubt
> it matters much either way (I'm actually tempted to suggest we bump
> the minimum curl version there; there's a ton of #ifdef cruft going
> back to 2002-era versions of libcurl).
We should switch to warning all the time since this series adds in
default whitelisted/blacklisted protocols anyways.
--
Brandon Williams
^ permalink raw reply related
* Re* git pull --rebase should use fast forward merge if possible
From: Junio C Hamano @ 2016-12-01 17:59 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: neuling, Stefan Beller
In-Reply-To: <CAPc5daURyXO6-yaOWPhvvdS8Dr5psEEc8MVP4wQJ_AuxyZraRg@mail.gmail.com>
Junio C Hamano <gitster@pobox.com> writes:
> die_no_merge_candidates() would have triggered, I would imagine.
>
> Note that I won't be applying this without test updates and proper log message,
> so no need to worry about the style yet ;-)
This time with a bit of explanation in the log and a trivial test.
I am no longer sure if this is a good idea myself, though. The
trivial case the new test covers is not interesting, even though it
may be worth protecting the behaviour with the test. What's more
interesting to think about is what should happen in various corner
cases. E.g. what should happen when there are local changes that
would conflict with the fast-forwarding? what should happen if the
user has rebase.autostash set in such a case? etc.
-- >8 --
Subject: [PATCH] pull: fast-forward "pull --rebase=true"
"git pull --rebase" always runs "git rebase" after fetching the
commit to serve as the new base, even when the new base is a
descendant of the current HEAD, i.e. we haven't done any work.
In such a case, we can instead fast-forward to the new base without
invoking the rebase process.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/pull.c | 22 ++++++++++++++++++----
t/t5520-pull.sh | 17 +++++++++++++++++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index bf3fd3f9c8..2a41d415b2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -878,10 +878,24 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (merge_heads.nr > 1)
die(_("Cannot merge multiple branches into empty head."));
return pull_into_void(*merge_heads.sha1, curr_head);
- } else if (opt_rebase) {
- if (merge_heads.nr > 1)
- die(_("Cannot rebase onto multiple branches."));
+ }
+ if (opt_rebase && merge_heads.nr > 1)
+ die(_("Cannot rebase onto multiple branches."));
+
+ if (opt_rebase) {
+ struct commit_list *list = NULL;
+ struct commit *merge_head, *head;
+
+ head = lookup_commit_reference(orig_head);
+ commit_list_insert(head, &list);
+ merge_head = lookup_commit_reference(merge_heads.sha1[0]);
+ if (is_descendant_of(merge_head, list)) {
+ /* we can fast-forward this without invoking rebase */
+ opt_ff = "--ff-only";
+ return run_merge();
+ }
return run_rebase(curr_head, *merge_heads.sha1, rebase_fork_point);
- } else
+ } else {
return run_merge();
+ }
}
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a0013ee32f..7887b6d97b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -237,6 +237,23 @@ test_expect_success '--rebase' '
test new = "$(git show HEAD:file2)"
'
+test_expect_success '--rebase fast forward' '
+ git reset --hard before-rebase &&
+ git checkout -b ff &&
+ echo another modification >file &&
+ git commit -m third file &&
+
+ git checkout to-rebase &&
+ git pull --rebase . ff &&
+ test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" &&
+
+ # The above only validates the result. Did we actually bypass rebase?
+ git reflog -1 >reflog.actual &&
+ sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
+ echo "OBJID HEAD@{0}: pull --rebase . ff: Fast-forward" >reflog.expected &&
+ test_cmp reflog.expected reflog.fuzzy
+'
+
test_expect_success '--rebase fails with multiple branches' '
git reset --hard before-rebase &&
test_must_fail git pull --rebase . copy master 2>err &&
--
2.11.0-192-gbadfaabe38
^ permalink raw reply related
* Re: [PATCH v6 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-12-01 17:51 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, peff, sbeller, jonathantanmy, gitster
In-Reply-To: <c6b2ddad-ac09-3457-8289-88a3f52b7e4b@kdbg.org>
On 12/01, Johannes Sixt wrote:
> Am 01.12.2016 um 02:28 schrieb Brandon Williams:
> >+ git init "su:b" &&
>
> Don't do that. Colons in file names won't work on Windows.
>
> -- Hannes
>
This test is needed to see if the code still works with filenames that
contain colons. Is there a way to mark the test to not run on windows?
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v6 0/6] recursively grep across submodules
From: Brandon Williams @ 2016-12-01 17:45 UTC (permalink / raw)
To: Jeff King; +Cc: git, sbeller, jonathantanmy, gitster
In-Reply-To: <20161201042228.ynug33mcsqkdbuoe@sigill.intra.peff.net>
On 11/30, Jeff King wrote:
> On Wed, Nov 30, 2016 at 05:28:28PM -0800, Brandon Williams wrote:
>
> > v6 fixes a race condition which existed in the 'is_submodule_populated'
> > function. Instead of calling 'resolve_gitdir' to check for the existance of a
> > .git file/directory, use 'stat'. 'resolve_gitdir' calls 'chdir' which can
> > affect other running threads trying to load thier files into a buffer in
> > memory.
>
> This one passes my stress-test for t7814 (though I imagine you already
> knew that).
>
> I tried to think of things that could go wrong by using a simple stat()
> instead of resolve_gitdir(). They should only differ when ".git" for
> some reason does not point to a git repository. My initial thought is
> that this might be more vocal about errors, because the child process
> will complain. But actually, the original would already die if the
> ".git" file is funny, so we were pretty vocal already.
>
> I also wondered whether the sub-process might skip a bogus ".git" file
> and keep looking upward in the filesystem tree (which would confusingly
> end up back in the super-project!). But it looks like we bail hard when
> we see a ".git" file but it's bogus. Which is probably a good thing in
> general for submodules.
>
> I'm not sure any of that is actually even worth worrying about, as such
> a setup is broken by definition. I just wanted to think it through as a
> devil's advocate, and even that seems pretty reasonable.
>
> -Peff
Yeah I was trying to think through these scenarios myself last night.
And like you found it seemed alright to let the child process deal with
the .git file/dir as long as once actually exists at that path. If one
didn't then there would be the possibility that we ended up back at the
superproject, which would result in an infinite loop. And yeah if the
.git file doesn't resolve to anything sensible then the user probably
mangled their repository somehow anyways.
Thanks again for all the help!
--
Brandon Williams
^ permalink raw reply
* Re: [ANNOUNCE] Git for Windows 2.11.0
From: Johannes Schindelin @ 2016-12-01 17:05 UTC (permalink / raw)
To: stefan.naewe; +Cc: git-for-windows, git
In-Reply-To: <77d1fa5d-869c-546e-357b-cd1e6ffee48d@atlas-elektronik.com>
Hi Stefan,
On Thu, 1 Dec 2016, stefan.naewe@atlas-elektronik.com wrote:
> Am 01.12.2016 um 13:31 schrieb Johannes Schindelin:
>
> > * Support has been added to generate project files for Visual Studio
> > 2010 and later.
>
> That's not really a new feature of Git-for-Windows, is it ?
Yes it is. We had a script to generate project files for Visual Studio <
2008, but they were broken.
This script has been partially fixed, and support has been added to
generate the .vcxproj used by Visual Studio 2010 and later.
Further, compile errors have been addressed.
After that, I worked on being able to run the tests in a regular Git Bash
(i.e. *without* installing Git for Windows' SDK, just a regular Git for
Windows will do).
Took a lot of effort and time, too,
Johannes
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: René Scharfe @ 2016-12-01 16:31 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <fc602a66-a06c-203e-b50b-55fd7b258b54@web.de>
Am 01.12.2016 um 17:26 schrieb René Scharfe:
> The function qsort_s() was introduced with C11 Annex K; it provides the
> ability to pass a context pointer to the comparison function, supports
> the convention of using a NULL pointer for an empty array and performs a
> few safety checks.
>
> Add an implementation based on compat/qsort.c for platforms that lack a
> native standards-compliant qsort_s() (i.e. basically everyone). It
> doesn't perform the full range of possible checks: It uses size_t
> instead of rsize_t and doesn't check nmemb and size against RSIZE_MAX
> because we probably don't have the restricted size type defined. For
> the same reason it returns int instead of errno_t.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> Makefile | 10 ++++++++
> compat/qsort_s.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> git-compat-util.h | 6 +++++
> 3 files changed, 85 insertions(+)
> create mode 100644 compat/qsort_s.c
And here's the diff for compat/qsort_s.c with copy detection (-C -C):
---
compat/{qsort.c => qsort_s.c} | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/compat/qsort.c b/compat/qsort_s.c
similarity index 62%
copy from compat/qsort.c
copy to compat/qsort_s.c
index 7d071afb7..52d1f0a73 100644
--- a/compat/qsort.c
+++ b/compat/qsort_s.c
@@ -3,11 +3,12 @@
/*
* A merge sort implementation, simplified from the qsort implementation
* by Mike Haertel, which is a part of the GNU C Library.
+ * Added context pointer, safety checks and return value.
*/
static void msort_with_tmp(void *b, size_t n, size_t s,
- int (*cmp)(const void *, const void *),
- char *t)
+ int (*cmp)(const void *, const void *, void *),
+ char *t, void *ctx)
{
char *tmp;
char *b1, *b2;
@@ -21,13 +22,13 @@ static void msort_with_tmp(void *b, size_t n, size_t s,
b1 = b;
b2 = (char *)b + (n1 * s);
- msort_with_tmp(b1, n1, s, cmp, t);
- msort_with_tmp(b2, n2, s, cmp, t);
+ msort_with_tmp(b1, n1, s, cmp, t, ctx);
+ msort_with_tmp(b2, n2, s, cmp, t, ctx);
tmp = t;
while (n1 > 0 && n2 > 0) {
- if (cmp(b1, b2) <= 0) {
+ if (cmp(b1, b2, ctx) <= 0) {
memcpy(tmp, b1, s);
tmp += s;
b1 += s;
@@ -44,19 +45,25 @@ static void msort_with_tmp(void *b, size_t n, size_t s,
memcpy(b, t, (n - n2) * s);
}
-void git_qsort(void *b, size_t n, size_t s,
- int (*cmp)(const void *, const void *))
+int git_qsort_s(void *b, size_t n, size_t s,
+ int (*cmp)(const void *, const void *, void *), void *ctx)
{
const size_t size = st_mult(n, s);
char buf[1024];
+ if (!n)
+ return 0;
+ if (!b || !cmp)
+ return -1;
+
if (size < sizeof(buf)) {
/* The temporary array fits on the small on-stack buffer. */
- msort_with_tmp(b, n, s, cmp, buf);
+ msort_with_tmp(b, n, s, cmp, buf, ctx);
} else {
/* It's somewhat large, so malloc it. */
char *tmp = xmalloc(size);
- msort_with_tmp(b, n, s, cmp, tmp);
+ msort_with_tmp(b, n, s, cmp, tmp, ctx);
free(tmp);
}
+ return 0;
}
^ permalink raw reply related
* [PATCH 3/3] string-list: use QSORT_S in string_list_sort()
From: René Scharfe @ 2016-12-01 16:29 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <3083fbf7-d67e-77e4-e05f-94a7e7e15eba@web.de>
Pass the comparison function to cmp_items() via the context parameter of
qsort_s() instead of using a global variable. That allows calling
string_list_sort() from multiple parallel threads.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
string-list.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/string-list.c b/string-list.c
index 8c83cac18..45016ad86 100644
--- a/string-list.c
+++ b/string-list.c
@@ -211,21 +211,18 @@ struct string_list_item *string_list_append(struct string_list *list,
list->strdup_strings ? xstrdup(string) : (char *)string);
}
-/* Yuck */
-static compare_strings_fn compare_for_qsort;
-
-/* Only call this from inside string_list_sort! */
-static int cmp_items(const void *a, const void *b)
+static int cmp_items(const void *a, const void *b, void *ctx)
{
+ compare_strings_fn cmp = ctx;
const struct string_list_item *one = a;
const struct string_list_item *two = b;
- return compare_for_qsort(one->string, two->string);
+ return cmp(one->string, two->string);
}
void string_list_sort(struct string_list *list)
{
- compare_for_qsort = list->cmp ? list->cmp : strcmp;
- QSORT(list->items, list->nr, cmp_items);
+ QSORT_S(list->items, list->nr, cmp_items,
+ list->cmp ? list->cmp : strcmp);
}
struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
--
2.11.0
^ permalink raw reply related
* [PATCH 2/3] add QSORT_S
From: René Scharfe @ 2016-12-01 16:28 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <3083fbf7-d67e-77e4-e05f-94a7e7e15eba@web.de>
Add the macro QSORT_S, a convenient wrapper for qsort_s() that infers
the size of the array elements and dies on error.
Basically all possible errors are programming mistakes (passing NULL as
base of a non-empty array, passing NULL as comparison function,
out-of-bounds accesses), so terminating the program should be acceptable
for most callers.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
git-compat-util.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index d25f0bd4c..b707dd880 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -994,6 +994,11 @@ int git_qsort_s(void *base, size_t nmemb, size_t size,
#define qsort_s git_qsort_s
#endif
+#define QSORT_S(base, n, compar, ctx) do { \
+ if (qsort_s((base), (n), sizeof(*(base)), compar, ctx)) \
+ die("BUG: qsort_s() failed"); \
+} while (0)
+
#ifndef REG_STARTEND
#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
#endif
--
2.11.0
^ permalink raw reply related
* [PATCH 1/3] compat: add qsort_s()
From: René Scharfe @ 2016-12-01 16:26 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <3083fbf7-d67e-77e4-e05f-94a7e7e15eba@web.de>
The function qsort_s() was introduced with C11 Annex K; it provides the
ability to pass a context pointer to the comparison function, supports
the convention of using a NULL pointer for an empty array and performs a
few safety checks.
Add an implementation based on compat/qsort.c for platforms that lack a
native standards-compliant qsort_s() (i.e. basically everyone). It
doesn't perform the full range of possible checks: It uses size_t
instead of rsize_t and doesn't check nmemb and size against RSIZE_MAX
because we probably don't have the restricted size type defined. For
the same reason it returns int instead of errno_t.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Makefile | 10 ++++++++
compat/qsort_s.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 6 +++++
3 files changed, 85 insertions(+)
create mode 100644 compat/qsort_s.c
diff --git a/Makefile b/Makefile
index f53fcc90d..2245fd95d 100644
--- a/Makefile
+++ b/Makefile
@@ -279,6 +279,9 @@ all::
# is a simplified version of the merge sort used in glibc. This is
# recommended if Git triggers O(n^2) behavior in your platform's qsort().
#
+# Define HAVE_QSORT_S if your platform provides a qsort_s() that's
+# compatible with the one described in C11 Annex K.
+#
# Define UNRELIABLE_FSTAT if your system's fstat does not return the same
# information on a not yet closed file that lstat would return for the same
# file after it was closed.
@@ -1423,6 +1426,13 @@ ifdef INTERNAL_QSORT
COMPAT_CFLAGS += -DINTERNAL_QSORT
COMPAT_OBJS += compat/qsort.o
endif
+
+ifdef HAVE_QSORT_S
+ COMPAT_CFLAGS += -DHAVE_QSORT_S
+else
+ COMPAT_OBJS += compat/qsort_s.o
+endif
+
ifdef RUNTIME_PREFIX
COMPAT_CFLAGS += -DRUNTIME_PREFIX
endif
diff --git a/compat/qsort_s.c b/compat/qsort_s.c
new file mode 100644
index 000000000..52d1f0a73
--- /dev/null
+++ b/compat/qsort_s.c
@@ -0,0 +1,69 @@
+#include "../git-compat-util.h"
+
+/*
+ * A merge sort implementation, simplified from the qsort implementation
+ * by Mike Haertel, which is a part of the GNU C Library.
+ * Added context pointer, safety checks and return value.
+ */
+
+static void msort_with_tmp(void *b, size_t n, size_t s,
+ int (*cmp)(const void *, const void *, void *),
+ char *t, void *ctx)
+{
+ char *tmp;
+ char *b1, *b2;
+ size_t n1, n2;
+
+ if (n <= 1)
+ return;
+
+ n1 = n / 2;
+ n2 = n - n1;
+ b1 = b;
+ b2 = (char *)b + (n1 * s);
+
+ msort_with_tmp(b1, n1, s, cmp, t, ctx);
+ msort_with_tmp(b2, n2, s, cmp, t, ctx);
+
+ tmp = t;
+
+ while (n1 > 0 && n2 > 0) {
+ if (cmp(b1, b2, ctx) <= 0) {
+ memcpy(tmp, b1, s);
+ tmp += s;
+ b1 += s;
+ --n1;
+ } else {
+ memcpy(tmp, b2, s);
+ tmp += s;
+ b2 += s;
+ --n2;
+ }
+ }
+ if (n1 > 0)
+ memcpy(tmp, b1, n1 * s);
+ memcpy(b, t, (n - n2) * s);
+}
+
+int git_qsort_s(void *b, size_t n, size_t s,
+ int (*cmp)(const void *, const void *, void *), void *ctx)
+{
+ const size_t size = st_mult(n, s);
+ char buf[1024];
+
+ if (!n)
+ return 0;
+ if (!b || !cmp)
+ return -1;
+
+ if (size < sizeof(buf)) {
+ /* The temporary array fits on the small on-stack buffer. */
+ msort_with_tmp(b, n, s, cmp, buf, ctx);
+ } else {
+ /* It's somewhat large, so malloc it. */
+ char *tmp = xmalloc(size);
+ msort_with_tmp(b, n, s, cmp, tmp, ctx);
+ free(tmp);
+ }
+ return 0;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092..d25f0bd4c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -988,6 +988,12 @@ static inline void sane_qsort(void *base, size_t nmemb, size_t size,
qsort(base, nmemb, size, compar);
}
+#ifndef HAVE_QSORT_S
+int git_qsort_s(void *base, size_t nmemb, size_t size,
+ int (*compar)(const void *, const void *, void *), void *ctx);
+#define qsort_s git_qsort_s
+#endif
+
#ifndef REG_STARTEND
#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
#endif
--
2.11.0
^ permalink raw reply related
* [PATCH 0/3] string-list: make string_list_sort() reentrant
From: René Scharfe @ 2016-12-01 16:24 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
Use qsort_s() from C11 Annex K to make string_list_sort() safer, in
particular when called from parallel threads.
compat: add qsort_s()
add QSORT_S
string-list: use QSORT_S in string_list_sort()
Makefile | 10 ++++++++
compat/qsort_s.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 11 +++++++++
string-list.c | 13 ++++-------
4 files changed, 95 insertions(+), 8 deletions(-)
create mode 100644 compat/qsort_s.c
--
2.11.0
^ permalink raw reply
* Re: [PATCH 2/6] http: always update the base URL for redirects
From: Ramsay Jones @ 2016-12-01 16:02 UTC (permalink / raw)
To: Jeff King, git; +Cc: Jann Horn
In-Reply-To: <20161201090414.zgz7pimgpctghbwu@sigill.intra.peff.net>
On 01/12/16 09:04, Jeff King wrote:
> If a malicious server redirects the initial ref
> advertisement, it may be able to leak sha1s from other,
> unrelated servers that the client has access to. For
> example, imagine that Alice is a git user, she has access to
> a private repository on a server hosted by Bob, and Mallory
> runs a malicious server and wants to find out about Bob's
> private repository.
>
> Mallory asks Alice to clone an unrelated repository from her
-----------------------------------------------------------^^^
... from _him_ ? (ie Mallory)
> over HTTP. When Alice's client contacts Mallory's server for
> the initial ref advertisement, the server issues an HTTP
> redirect for Bob's server. Alice contacts Bob's server and
> gets the ref advertisement for the private repository. If
> there is anything to fetch, she then follows up by asking
> the server for one or more sha1 objects. But who is the
> server?
>
> If it is still Mallory's server, then Alice will leak the
> existence of those sha1s to her.
------------------------------^^^
... to _him_ ? (again Mallory)
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 4/6] http: make redirects more obvious
From: Ramsay Jones @ 2016-12-01 16:06 UTC (permalink / raw)
To: Jeff King, git; +Cc: Jann Horn
In-Reply-To: <20161201090428.pweq7slwujbne5hg@sigill.intra.peff.net>
On 01/12/16 09:04, Jeff King wrote:
> We instruct curl to always follow HTTP redirects. This is
> convenient, but it creates opportunities for malicious
> servers to create confusing situations. For instance,
> imagine Alice is a git user with access to a private
> repository on Bob's server. Mallory runs her own server and
Ahem, so Mallory is female? (-blush-) :(
ATB,
Ramsay Jones
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox