On Thu, Aug 15, 2024 at 03:19:50AM -0700, karthik nayak wrote: > shejialuo <shejialuo@xxxxxxxxx> writes: > > > Hi All: > > > > We have already set up the infrastructure of the ref consistency. > > However, we have only add ref name check when establishing the > > infrastructure in below: > > > > https://lore.kernel.org/git/ZrSqMmD-quQ18a9F@ArchLinux.localdomain/ > > > > Actually, we already have a patch here which has already implemented the > > ref content consistency check. But during the review process, we have > > encountered some problems. The intention of this RFC is to make sure > > what content we should check and also to what extend. > > > > I conclude the following info: > > > > 1. For the regular ref which has a trailing garbage, we should warn the > > user. This is the most simplest situation, we could reply on > > "parse_loose_ref_content" to do this. > > 2. For the symref, we could also rely on "parse_loose_ref_content" to > > get the "pointee", and check the location of the "pointee", check the > > name of the "pointee" and the file type of the "pointee". > > 3. FOr the symbolic ref, we could follow the idea of 2. > > > > Just to understand clearly, when you're talking about 'symbolic ref' you > are referring to symbolic links? > I am sorry about this. It's symbolic links here. > > But Patrick gives a question here: > > > >> In case the ref ends with a newline, should we check that the next > >> character is `\0`? Otherwise, it may contain multiple lines, which is > >> not allowed for a normal ref. > >> > >> Also, shouldn't the ref always end with a newline? > > > > For symref, I guess we have no spec here. From my experiments, a symref > > could have a newline or no newline, even multiple newlines. And also > > symref could have multiple spaces. But the following is a bad symref > > > > ref: refs/heads/main garbage > > > > I think we should fully discuss what we should check here. Thus I will > > implement the code. > > > > Agreed, in refs/files-backend.c:create_symref_lock, we write symrefs as > "ref: %s\n" so it makes sense to validate that there is nothing extra. Yes, we should do this. I will implement the code and the send the patches to the mailing list. Thanks
