Re: [PATCH v4 4/5] ref: add symref content check for files backend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 18, 2024 at 01:19:13PM -0700, Junio C Hamano wrote:
> shejialuo <shejialuo@xxxxxxxxx> writes:
> 
> Expect that people do not read the body of the message as completing
> a paragrpah the title started.  I.e. ...
> 
> > We have already introduced the checks for regular refs. There is no need
> > to check the consistency of the target which the symref points to.
> > Instead, we just need to check the content of the symref itself.
> 
> ... this needs a bit of preamble, like
> 
>     We have code that check regular ref contents, but we do not yet
>     check contents of symbolic refs.
> 

Thanks, I will improve this in the next version.

> > A regular file is accepted as a textual symref if it begins with
> > "ref:", followed by zero or more whitespaces, followed by the full
> > refname, followed only by whitespace characters. We always write
> > a single SP after "ref:" and a single LF after the refname, but
> > third-party reimplementations of Git may have taken advantage of the
> > looser syntax. Put it more specific, we accept the following contents
> > of the symref:
> >
> > 1. "ref: refs/heads/master   "
> > 2. "ref: refs/heads/master   \n  \n"
> > 3. "ref: refs/heads/master\n\n"
> >
> > Thus, we could reuse "refMissingNewline" and "trailingRefContent"
> > FSCK_INFOs to do the same retroactive tightening as we introduce for
> > regular references.
> >
> > But we do not allow any other trailing garbage. The followings are bad
> > symref contents which will be reported as fsck error by "git-fsck(1)".
> 
> This description needs to be updated, as it is unclear if you are
> talking about errors we already detect, or if you are planning to
> update fsck to notice and report these errors.
> 

Yes, When I was writing this part, I felt a little painful to express my
words. I have thought how could I express the connection between the
current patch and the previous one.

> > And we will remember the untrimmed length of the "referent" and call
> > "strbuf_rtrim()" on "referent". Then, we will call "check_refname_format"
> > to check whether the trimmed referent format is valid. If not, we will
> > report to the user that the symref points to referent which has invalid
> > format. If it is valid, we will compare the untrimmed length and trimmed
> > length, if they are not the same, we need to warn the user there is some
> > trailing garbage in the symref content.
> 
> That is an implementation detail of what you did.  But if the
> implementation were buggy and did not exactly what you intended to
> do, the above description gives no information to help others to fix
> it up so that it works as you intended it to work, because you do
> not explain it.
> 
> So what did you want to achieve in the third step (the first being
> "limit to refs/ hiararchy", the second being "no incomplete lines
> allowed")?
> 
>     Third, we want to make sure that the contents of a textual
>     symref MUST have a single LF after the target refname and
>     NOTHING ELSE.
> 
> or something.
> 


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux