| Message ID | 86e4ce0e8d9665b1c5a2b30173be4afffe0a0abd.1617929278.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Push negotiation | expand |
Jonathan Tan <jonathantanmy@google.com> writes: > In send_fetch_request(), "object-format" is written directly to the file > descriptor, as opposed to the other arguments, which are buffered. > Buffer "object-format" as well. "object-format" must be buffered; in > particular, it must appear after "command=fetch" in the request. > > This divergence was introduced in 4b831208bb ("fetch-pack: parse and > advertise the object-format capability", 2020-05-27), perhaps as an > oversight (the surrounding code at the point of this commit has already > been using a request buffer.) Good find. Did this actually resulted in a data corruption and that was how you discovered it? > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > fetch-pack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fetch-pack.c b/fetch-pack.c > index 6e68276aa3..2318ebe680 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -1251,7 +1251,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, > if (hash_algo_by_ptr(the_hash_algo) != hash_algo) > die(_("mismatched algorithms: client %s; server %s"), > the_hash_algo->name, hash_name); > - packet_write_fmt(fd_out, "object-format=%s", the_hash_algo->name); > + packet_buf_write(&req_buf, "object-format=%s", the_hash_algo->name); > } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1) { > die(_("the server does not support algorithm '%s'"), > the_hash_algo->name);
> Jonathan Tan <jonathantanmy@google.com> writes: > > > In send_fetch_request(), "object-format" is written directly to the file > > descriptor, as opposed to the other arguments, which are buffered. > > Buffer "object-format" as well. "object-format" must be buffered; in > > particular, it must appear after "command=fetch" in the request. > > > > This divergence was introduced in 4b831208bb ("fetch-pack: parse and > > advertise the object-format capability", 2020-05-27), perhaps as an > > oversight (the surrounding code at the point of this commit has already > > been using a request buffer.) > > Good find. Did this actually resulted in a data corruption and that > was how you discovered it? No, I did not see any data corruption. I discovered it because I was looking at this function, thinking that I needed to refactor it.
diff --git a/fetch-pack.c b/fetch-pack.c index 6e68276aa3..2318ebe680 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1251,7 +1251,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, if (hash_algo_by_ptr(the_hash_algo) != hash_algo) die(_("mismatched algorithms: client %s; server %s"), the_hash_algo->name, hash_name); - packet_write_fmt(fd_out, "object-format=%s", the_hash_algo->name); + packet_buf_write(&req_buf, "object-format=%s", the_hash_algo->name); } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1) { die(_("the server does not support algorithm '%s'"), the_hash_algo->name);
In send_fetch_request(), "object-format" is written directly to the file descriptor, as opposed to the other arguments, which are buffered. Buffer "object-format" as well. "object-format" must be buffered; in particular, it must appear after "command=fetch" in the request. This divergence was introduced in 4b831208bb ("fetch-pack: parse and advertise the object-format capability", 2020-05-27), perhaps as an oversight (the surrounding code at the point of this commit has already been using a request buffer.) Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- fetch-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)