C. Michael Pilato wrote on Fri, May 10, 2013 at 18:19:56 -0400:
> On 05/10/2013 04:51 PM, Daniel Shahaf wrote:
> > For issue #4340, we decided to block filenames containing \n in FSFS and
> > filenames containing control characters in libsvn_repos. (I agree with
> > that decision.)
> >
> > However, those changes are now nominated for backport towards 1.7.10 in
> > 1.7.x/STATUS, and I voted -0 on them, saying that "[the new] libsvn_repos
> > restrictions [are] not suitable for introduction in a patch release" ---
> > that is, that libsvn_repos-1.7.10 should not forbid creating fspaths that
> > contain control characters, because libsvn_repos-1.7.9 doesn't.
> >
> > Stefan asked to start a thread about that -0 vote, so here it is :)
>
> If you look at the "general idea" behind our versioning guidelines[1], the
> one relevant goal of the three presented is this one:
>
> "Upgrading/downgrading between different patch releases in the same
> MAJOR.MINOR line never breaks code."
>
Agreed.
> So long as we don't introduce new APIs or break the calling conventions of
> existing ones to accomplish this change, disallowing certain characters in
> repository paths does not affect the upgrade/downgrade-ability of the
Disagree. Adding a validation to 1.7.10 that didn't exist in 1.7.9
means that client code that worked with a 1.7.9 libsvn would stop
working[1] when run with 1.7.10 libsvn. That is precisely "Upgrading
within the same minor line never breaks code", isn't it?
> codebase. Further, we're merely guarding our entry points against these
> troublesome paths, not croaking when we find them already in the repository.
Agreed.
> As such, it's *not* the case that users find themselves with a new freedom
> in 1.7.10 that, if exercised, will cause a downgrade to 1.7.9 to render
> their repositories inaccessible.
>
Agreed again, downgrading would not cause a problem.
> In summary: I think the backport is fine.
>
> Concession: It could be argued that the no-"\n"-in-FSFS change is
> independent from the no-control-chars-in-libsvn_repos change. The former is
> trivially defensible as a bugfix to guard against a demonstrable data
> storage problem. The latter change is, IMO, the only possible controversial
> one for a patch release. While I think both backports are fine, I wouldn't
> strongly defend the latter.
>
I already voted +1 on the "No '\n' in FSFS" change; I only have concerns
about the "No control-chars in libsvn_repos" change.
>
Thanks for the feedback.
Cheers,
Daniel
[1] Specifically: svn_repos_fs_commit_txn() would start returning
an SVN_ERR_FS_PATH_SYNTAX error instead of succeeding.
Received on 2013-05-11 01:06:08 CEST