Do not evaluate ${username} and ${shardeduserid} in LabelTypes

Gerrit does ref pattern matching for any refs configured in
project.config. This includes username and shardeduserid. While that
makes sense for access control, it is confusing for labels.

Consider the following case:
- Label is configured on refs/heads/sandbox/${username}/*
- User A creates a change on User B's ref refs/heads/sandbox/B

We usually evaluate these patterns to match the current user. That would
mean that we get different labels depending on if user A or user B looks
at the change.

Alternatively, we could also evaluate the ref using the change owner,
but that would be wrong in the example as well.

This commit removes the option to have these magic operators in label
specs. Users who need this can use a wildcard instead.

Sandbox branches are rarely used and user-ref patterns in labels are also
not used very frequently. In fact, the way that Gerrit currently behaves
is not easy to understand I would doubt that people use user-ref
patterns on labels productively.

Change-Id: I8284a1b35edd0c108ce2e3a471993c1ead897b83
This commit is contained in:
Patrick Hiesel
2018-07-09 16:06:21 +02:00
parent 75aa75a39c
commit 7e50ec9e81
2 changed files with 14 additions and 3 deletions

View File

@@ -368,6 +368,9 @@ assign permissions for that label on a branch, but this permission is then
ignored if the label doesn't apply for that branch.
Additionally, the `branch` modifier has no effect when the submit rule
is customized in the rules.pl of the project or inherited from parent projects.
Branch can be a ref pattern similar to what is documented
link:access-control.html#reference[here], but must not contain `${username}` or
`${shardeduserid}`.
[[label_example]]
=== Example

View File

@@ -410,7 +410,15 @@ public class ProjectState {
r.add(l);
} else {
for (String refPattern : refs) {
if (RefConfigSection.isValid(refPattern) && match(destination, refPattern, user)) {
if (refPattern.contains("${")) {
logger.atWarning().log(
"Ref pattern for label %s in project %s contains illegal expanded parameters: %s."
+ " Ref pattern will be ignored.",
l, getName(), refPattern);
continue;
}
if (RefConfigSection.isValid(refPattern) && match(destination, refPattern)) {
r.add(l);
break;
}
@@ -558,7 +566,7 @@ public class ProjectState {
return new LabelTypes(Collections.unmodifiableList(all));
}
private boolean match(Branch.NameKey destination, String refPattern, CurrentUser user) {
return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), user);
private boolean match(Branch.NameKey destination, String refPattern) {
return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), null);
}
}