On Fri, Aug 23, 2024 at 07:36:10AM +0200, Patrick Steinhardt wrote:
> On Thu, Aug 22, 2024 at 08:42:02PM +0800, shejialuo wrote:
> > On Thu, Aug 22, 2024 at 10:53:57AM +0200, Patrick Steinhardt wrote:
> >
> >
> > > > + if ((space_num || newline_num) && !isspace(*p)) {
> > > > + ret = fsck_report_ref(o, report,
> > > > + FSCK_MSG_BAD_REF_CONTENT,
> > > > + "contains non-null garbage");
> > > > + goto out;
> > > > + }
> > > > +
> > > > + if (*p == '\n') {
> > > > + newline_num++;
> > > > + } else if (*p == ' ') {
> > > > + space_num++;
> > > > + }
> > > > + p++;
> > > > + }
> > >
> > > Can't we replace this with a single `strchr('\n')` call to check for the
> > > newline and then verify that the next character is a `\0`? The check for
> > > spaces would then be handled by `check_refname_format()`.
> > >
> >
> > We cannot. Think about this situation.
> >
> > "ref: refs/heads/master \n "
> >
> > We find that the next character of '\n' is not '\0'. Then we leave it to
> > "check_refname_format". But "check_refname_format" will report an error
> > here, but this is an allowed symref.
>
> Wouldn't it be correct to warn about this? To me the above very much
> looks like garbage after the refname, same like we'd also warn about
> such garbage for direct refs.
>
Yes, we should warn about this. But only null-garbage is allowed for
symref. The following situation is bad:
"ref: refs/heads/master \n garbage\n"
We should report error here, from my perspective, it's a FATAL ERROR.
However, let's decide how to do this when we know what fsck error level
we should set.
> Patrick
