On Wed, Jul 31, 2024 at 8:35 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Christian Couder <christian.couder@xxxxxxxxx> writes: > > > 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. > > A malicous server can swich name and url correspondence. The URLs > this repository uses to lazily fetch missing objects from are the > only thing that matters, and it does not matter what name the server > calls these URLs as, I am not sure what value, if any, KnownName has, > other than adding a potential security hole. In a corporate setup where clients and servers trust each other to not switch names and URLs, it could be valuable to still have a bit of control in a simple way, for example: - if servers use many promisor remotes, but clients should only use a subset of them, or: - if the URLs used by clients should not be the same as the URLs used by servers In version 2, I have updated the "promisor.acceptFromServer" documentation and the commit message of this patch to better explain cases where the new "KnownName" and "KnownUrl" could be useful. > > 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. > > This makes sense, especially if we had updates to documents I > suggested in my review of [3/4]. If the side effect of "accepting" > a suggested promisor remote were to only use it as a promisor remote > on this side, there is no reason to "accept" the same thing again, > but because the main effect at the protocol level of "accepting" is > to affect the behaviour of the server in such a way that it is now > allowed to omit objects that are requested but would be available > lazily from the promisor remotes in the response, we _do_ need to > be able to respond with the promisor remotes we are willing to and > have been using. Yeah, it is better to let the server know. > This iteration does not seem to have the true server side support to > slim its response by omitting objects that are available elsewhere, Yeah, in version 2, the commit message of patch 3/4 has been improved to say that implementation of this case, which would require S to omit in its response the objects available on X, is left for future improvement. > but I agree that it is a good approach to get the protocol support > right. Thanks.
