git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: peff@peff.net
Cc: jonathantanmy@google.com, git@vger.kernel.org, steadmon@google.com
Subject: Re: [PATCH 8/8] remote-curl: in v2, fill credentials if needed
Date: Mon, 11 Feb 2019 11:20:54 -0800	[thread overview]
Message-ID: <20190211192054.80935-1-jonathantanmy@google.com> (raw)
In-Reply-To: <20190206212928.GB12737@sigill.intra.peff.net>

> On Tue, Feb 05, 2019 at 04:21:22PM -0800, Jonathan Tan wrote:
> 
> > In post_rpc(), remote-curl calls credential_fill() if HTTP_REAUTH is
> > returned, but this is not true in proxy_request(). Do this in
> > proxy_request() too.
> 
> Can we do this as a general rule? If we look at the code in post_rpc(),
> there are two cases: when large_request is set and when it is not.
> 
> When it's not, we have the whole request in a buffer, and we can happily
> resend it.
> 
> But when it's not, we cannot restart it, because we'll have thrown away
> some of the data. So we send an initial probe_rpc() as a sanity check.
> If that works and we later get a 401 on the real request, we still fail
> anyway.
> 
> In the case of proxy_request(), we don't know ahead of time whether the
> request is large or not; we just proxy the data through. And we don't do
> the probe thing at all. So wouldn't we dropping some data for the
> follow-up request?

Thanks - I'll look into this. Maybe the best way is to somehow make the
v2 code path also use post_rpc() - I'll see if that's possible.

In the meantime, do you have any other opinions on the other patches,
besides introducing a prereq [1]? I don't have any strong opinions for
or against this, so I didn't reply, but I slightly prefer not having the
prereq so that test readers and authors don't need to juggle so many
variables in their heads.

If everything looks good, I'll suggest that we drop this patch from this
patch set for me to work on it independently. (Having said that, this
patch set is based on js/protocol-advertise-multi, which is still under
review, so it is not so urgent.)

[1] https://public-inbox.org/git/20190206213458.GC12737@sigill.intra.peff.net/

  reply	other threads:[~2019-02-11 19:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-06  0:21 [PATCH 0/8] Resend of GIT_TEST_PROTOCOL_VERSION patches Jonathan Tan
2019-02-06  0:21 ` [PATCH 1/8] tests: define GIT_TEST_PROTOCOL_VERSION Jonathan Tan
2019-02-06 21:58   ` Ævar Arnfjörð Bjarmason
2019-02-07  0:01     ` Jonathan Tan
2019-02-11 20:20   ` Jeff King
2019-02-06  0:21 ` [PATCH 2/8] tests: always test fetch of unreachable with v0 Jonathan Tan
2019-02-11 20:30   ` Jeff King
2019-02-14 19:58     ` Jonathan Tan
2019-02-21 13:49       ` Jeff King
2019-02-22 20:47         ` Junio C Hamano
2019-02-23 13:25           ` Jeff King
2019-02-06  0:21 ` [PATCH 3/8] t5503: fix overspecification of trace expectation Jonathan Tan
2019-02-06  0:21 ` [PATCH 4/8] t5512: compensate for v0 only sending HEAD symrefs Jonathan Tan
2019-02-06  0:21 ` [PATCH 5/8] t5700: only run with protocol version 1 Jonathan Tan
2019-02-06  0:21 ` [PATCH 6/8] tests: fix protocol version for overspecifications Jonathan Tan
2019-02-06  0:21 ` [PATCH 7/8] t5552: compensate for v2 filtering ref adv Jonathan Tan
2019-02-06  0:21 ` [PATCH 8/8] remote-curl: in v2, fill credentials if needed Jonathan Tan
2019-02-06 21:29   ` Jeff King
2019-02-11 19:20     ` Jonathan Tan [this message]
2019-02-11 20:38       ` Jeff King
2019-02-06 21:34 ` [PATCH 0/8] Resend of GIT_TEST_PROTOCOL_VERSION patches Jeff King
2019-02-06 21:52   ` Ævar Arnfjörð Bjarmason
2019-02-06 22:10     ` Jeff King
2019-02-06 22:20       ` Ævar Arnfjörð Bjarmason
2019-02-06 23:08         ` Jeff King
2019-02-07 10:49           ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190211192054.80935-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).