From a0610125bdd783cdbee3d4d714e69843b4791bbd Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 28 Jan 2013 22:09:07 -0800 Subject: [PATCH] Use IdString to wrap encoded strings in REST API calls Clients supply encoded strings as the id portion of REST API paths. These strings need to be decoded within the view code as some encodings are specially handled. Try to prevent misuse by wrapping strings in a type-safe IdString wrapper that forces developers to decode with ".get()" before using. The raw string can be obtained with ".encoded()", an obvious hint that the return value will still be URL encoded. This is used in only a handful of locations where special parsing must be performed. Change-Id: Ied5747d1fe8cc0f2dc24017f7aacc2fcab69a37c --- .../extensions/restapi/AcceptsCreate.java | 2 +- .../gerrit/extensions/restapi/IdString.java | 68 +++++++++++++++++++ .../restapi/ResourceNotFoundException.java | 5 ++ .../extensions/restapi/RestCollection.java | 2 +- .../gerrit/httpd/restapi/RestApiServlet.java | 26 ++++--- .../server/account/AccountsCollection.java | 8 +-- .../gerrit/server/account/Capabilities.java | 10 +-- .../server/change/ChangesCollection.java | 5 +- .../google/gerrit/server/change/Drafts.java | 6 +- .../gerrit/server/change/Reviewers.java | 7 +- .../gerrit/server/change/Revisions.java | 7 +- .../gerrit/server/group/CreateGroup.java | 3 +- .../gerrit/server/group/GroupsCollection.java | 25 ++++--- .../group/IncludedGroupsCollection.java | 10 +-- .../server/group/MembersCollection.java | 9 +-- .../server/plugins/PluginsCollection.java | 12 ++-- .../server/project/DashboardsCollection.java | 15 ++-- .../gerrit/server/project/GetDashboard.java | 19 +++++- .../server/project/ProjectsCollection.java | 6 +- .../server/project/SetDefaultDashboard.java | 7 +- 20 files changed, 179 insertions(+), 73 deletions(-) create mode 100644 gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/IdString.java diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AcceptsCreate.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AcceptsCreate.java index db5f89d12f..506f281d34 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AcceptsCreate.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AcceptsCreate.java @@ -30,5 +30,5 @@ public interface AcceptsCreate

{ * into the newly returned view object, as it will not be passed. * @throws RestApiException the view cannot be constructed. */ - RestModifyView create(P parent, String id) throws RestApiException; + RestModifyView create(P parent, IdString id) throws RestApiException; } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/IdString.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/IdString.java new file mode 100644 index 0000000000..f987425721 --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/IdString.java @@ -0,0 +1,68 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.extensions.restapi; + +/** + * Resource identifier split out from a URL. + *

+ * Identifiers are URL encoded and usually need to be decoded. + */ +public class IdString { + /** Construct an identifier from an already encoded string. */ + public static IdString fromUrl(String id) { + return new IdString(id); + } + + private final String urlEncoded; + + private IdString(String s) { + urlEncoded = s; + } + + /** @return the decoded value of the string. */ + public String get() { + return Url.decode(urlEncoded); + } + + /** @return true if the string is the empty string. */ + public boolean isEmpty() { + return urlEncoded.isEmpty(); + } + + /** @return the original URL encoding supplied by the client. */ + public String encoded() { + return urlEncoded; + } + + @Override + public int hashCode() { + return urlEncoded.hashCode(); + } + + @Override + public boolean equals(Object other) { + if (other instanceof IdString) { + return urlEncoded.equals(((IdString) other).urlEncoded); + } else if (other instanceof String) { + return urlEncoded.equals(other); + } + return false; + } + + @Override + public String toString() { + return encoded(); + } +} diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/ResourceNotFoundException.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/ResourceNotFoundException.java index 84e8f5b7ae..aa891c9404 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/ResourceNotFoundException.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/ResourceNotFoundException.java @@ -26,4 +26,9 @@ public class ResourceNotFoundException extends RestApiException { public ResourceNotFoundException(String id) { super(id); } + + /** @param id portion of the resource URI that does not exist. */ + public ResourceNotFoundException(IdString id) { + super(id.get()); + } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestCollection.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestCollection.java index 56ff5558e9..96d0dbf129 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestCollection.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestCollection.java @@ -83,7 +83,7 @@ public interface RestCollection

* @throws Exception if the implementation had any errors converting to a * resource handle. This results in an HTTP 500 Internal Server Error. */ - R parse(P parent, String id) throws ResourceNotFoundException, Exception; + R parse(P parent, IdString id) throws ResourceNotFoundException, Exception; /** * Get the views that support this collection. diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 67125e640b..acdfbaad41 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -47,6 +47,7 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.DefaultInput; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.PreconditionFailedException; import com.google.gerrit.extensions.restapi.PutInput; @@ -178,7 +179,7 @@ public class RestApiServlet extends HttpServlet { int status = SC_OK; checkUserSession(req); - List path = splitPath(req); + List path = splitPath(req); RestCollection rc = members.get(); checkAccessAnnotations(rc.getClass()); @@ -195,7 +196,7 @@ public class RestApiServlet extends HttpServlet { throw new MethodNotAllowedException(); } } else { - String id = path.remove(0); + IdString id = path.remove(0); try { rsrc = rc.parse(rsrc, id); checkPreconditions(req, rsrc); @@ -234,7 +235,7 @@ public class RestApiServlet extends HttpServlet { } break; } else { - String id = path.remove(0); + IdString id = path.remove(0); try { rsrc = c.parse(rsrc, id); checkPreconditions(req, rsrc); @@ -654,10 +655,12 @@ public class RestApiServlet extends HttpServlet { private RestView view( RestCollection rc, - String method, List path) throws ResourceNotFoundException, + String method, List path) throws ResourceNotFoundException, MethodNotAllowedException, AmbiguousViewException { DynamicMap> views = rc.views(); - final String projection = path.isEmpty() ? "/" : path.remove(0); + final IdString projection = path.isEmpty() + ? IdString.fromUrl("/") + : path.remove(0); if (!path.isEmpty()) { // If there are path components still remaining after this projection // is chosen, look for the projection based upon GET as the method as @@ -707,20 +710,25 @@ public class RestApiServlet extends HttpServlet { } } - private static List splitPath(HttpServletRequest req) { + private static List splitPath(HttpServletRequest req) { String path = req.getPathInfo(); if (Strings.isNullOrEmpty(path)) { return Collections.emptyList(); } - List out = Lists.newArrayList(Splitter.on('/').split(path)); + List out = Lists.newArrayList(); + for (String p : Splitter.on('/').split(path)) { + out.add(IdString.fromUrl(p)); + } if (out.size() > 0 && out.get(out.size() - 1).isEmpty()) { out.remove(out.size() - 1); } return out; } - private static List splitProjection(String projection) { - return Lists.newArrayList(Splitter.on('~').limit(2).split(projection)); + private static List splitProjection(IdString projection) { + List p = Lists.newArrayListWithCapacity(2); + Iterables.addAll(p, Splitter.on('~').limit(2).split(projection.get())); + return p; } private void checkUserSession(HttpServletRequest req) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java index 8eae257805..83e7c858b3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java @@ -17,11 +17,11 @@ package com.google.gerrit.server.account; import com.google.common.collect.Iterables; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.TopLevelResource; -import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; @@ -54,11 +54,11 @@ public class AccountsCollection implements } @Override - public AccountResource parse(TopLevelResource root, String id) + public AccountResource parse(TopLevelResource root, IdString id) throws ResourceNotFoundException, AuthException, OrmException { CurrentUser user = self.get(); - if ("self".equals(id)) { + if (id.equals("self")) { if (user instanceof IdentifiedUser) { return new AccountResource((IdentifiedUser) user); } else if (user instanceof AnonymousUser) { @@ -68,7 +68,7 @@ public class AccountsCollection implements } } - Set matches = resolver.findAll(Url.decode(id)); + Set matches = resolver.findAll(id.get()); if (matches.size() != 1) { throw new ResourceNotFoundException(id); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Capabilities.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Capabilities.java index aa8dfcfe69..38e50132e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Capabilities.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Capabilities.java @@ -18,6 +18,7 @@ import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.server.CurrentUser; @@ -47,17 +48,18 @@ class Capabilities implements } @Override - public Capability parse(AccountResource parent, String id) + public Capability parse(AccountResource parent, IdString id) throws ResourceNotFoundException, AuthException { if (self.get() != parent.getUser() && !self.get().getCapabilities().canAdministrateServer()) { throw new AuthException("restricted to administrator"); } + String name = id.get(); CapabilityControl cap = parent.getUser().getCapabilities(); - if (cap.canPerform(id) - || (cap.canAdministrateServer() && GlobalCapability.isCapability(id))) { - return new AccountResource.Capability(parent.getUser(), id); + if (cap.canPerform(name) + || (cap.canAdministrateServer() && GlobalCapability.isCapability(name))) { + return new AccountResource.Capability(parent.getUser(), name); } throw new ResourceNotFoundException(id); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java index 313ff8cbf3..0100ebcf18 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.change; import com.google.common.collect.ImmutableList; import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestView; @@ -68,10 +69,10 @@ public class ChangesCollection implements } @Override - public ChangeResource parse(TopLevelResource root, String id) + public ChangeResource parse(TopLevelResource root, IdString id) throws ResourceNotFoundException, OrmException, UnsupportedEncodingException { - ParsedId p = new ParsedId(id); + ParsedId p = new ParsedId(id.encoded()); List changes = findChanges(p); if (changes.size() != 1) { throw new ResourceNotFoundException(id); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java index d3515ecb57..b8b6e7819b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Drafts.java @@ -17,9 +17,9 @@ package com.google.gerrit.server.change; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; -import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; @@ -57,10 +57,10 @@ class Drafts implements ChildCollection { } @Override - public DraftResource parse(RevisionResource rev, String id) + public DraftResource parse(RevisionResource rev, IdString id) throws ResourceNotFoundException, OrmException, AuthException { checkIdentifiedUser(); - String uuid = Url.decode(id); + String uuid = id.get(); for (PatchLineComment c : dbProvider.get().patchComments() .draftByPatchSetAuthor( rev.getPatchSet().getId(), diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java index ac89b2a591..267f357c65 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java @@ -18,6 +18,7 @@ import com.google.common.collect.Sets; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Account; @@ -62,7 +63,7 @@ public class Reviewers implements } @Override - public ReviewerResource parse(ChangeResource rsrc, String id) + public ReviewerResource parse(ChangeResource rsrc, IdString id) throws OrmException, ResourceNotFoundException, AuthException { Account.Id accountId; if (id.equals("self")) { @@ -74,8 +75,8 @@ public class Reviewers implements } else { throw new ResourceNotFoundException(id); } - } else if (id.matches("^[0-9]+$")) { - accountId = Account.Id.parse(id); + } else if (id.get().matches("^[0-9]+$")) { + accountId = Account.Id.parse(id.get()); } else { throw new ResourceNotFoundException(id); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java index 970230b05c..3501351d54 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.change; import com.google.common.collect.Lists; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Change; @@ -52,10 +53,10 @@ class Revisions implements ChildCollection { } @Override - public RevisionResource parse(ChangeResource change, String id) - throws ResourceNotFoundException, Exception { + public RevisionResource parse(ChangeResource change, IdString id) + throws ResourceNotFoundException, OrmException { List match = Lists.newArrayListWithExpectedSize(2); - for (PatchSet ps : find(change, id)) { + for (PatchSet ps : find(change, id.get())) { Change.Id changeId = ps.getId().getParentKey(); if (changeId.equals(change.getChange().getId()) && change.getControl().isPatchVisible(ps, dbProvider.get())) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java index 577c38fea5..cc30b7312d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java @@ -26,6 +26,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.TopLevelResource; +import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.IdentifiedUser; @@ -106,7 +107,7 @@ class CreateGroup implements RestModifyView { private AccountGroup.Id owner(Input input) throws BadRequestException { if (input.ownerId != null) { try { - GroupResource rsrc = groups.parse(input.ownerId); + GroupResource rsrc = groups.parse(Url.decode(input.ownerId)); AccountGroup owner = GroupDescriptions.toAccountGroup(rsrc.getGroup()); if (owner == null) { throw new BadRequestException("ownerId must be internal group"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java index 2c1c91ae58..7d18f98ef1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java @@ -19,11 +19,11 @@ import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsCreate; import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.TopLevelResource; -import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; @@ -73,7 +73,7 @@ public class GroupsCollection implements } @Override - public GroupResource parse(TopLevelResource parent, String id) + public GroupResource parse(TopLevelResource parent, IdString id) throws ResourceNotFoundException, Exception { final CurrentUser user = self.get(); if (user instanceof AnonymousUser) { @@ -81,15 +81,14 @@ public class GroupsCollection implements } else if(!(user instanceof IdentifiedUser)) { throw new ResourceNotFoundException(id); } - return parse(id); + return parse(id.get()); } - GroupResource parse(String urlId) throws ResourceNotFoundException { - String id = Url.decode(urlId); + GroupResource parse(String id) throws ResourceNotFoundException { try { AccountGroup.UUID uuid = new AccountGroup.UUID(id); if (groupBackend.handles(uuid)) { - return check(urlId, groupControlFactory.controlFor(uuid)); + return check(id, groupControlFactory.controlFor(uuid)); } } catch (NoSuchGroupException noSuchGroup) { } @@ -98,7 +97,7 @@ public class GroupsCollection implements if (id.matches("^[1-9][0-9]*$")) { try { AccountGroup.Id legacyId = AccountGroup.Id.parse(id); - return check(urlId, groupControlFactory.controlFor(legacyId)); + return check(id, groupControlFactory.controlFor(legacyId)); } catch (IllegalArgumentException invalidId) { } catch (NoSuchGroupException e) { } @@ -108,26 +107,26 @@ public class GroupsCollection implements GroupReference ref = GroupBackends.findExactSuggestion(groupBackend, id); if (ref != null) { try { - return check(urlId, groupControlFactory.controlFor(ref.getUUID())); + return check(id, groupControlFactory.controlFor(ref.getUUID())); } catch (NoSuchGroupException e) { } } - throw new ResourceNotFoundException(urlId); + throw new ResourceNotFoundException(id); } - private GroupResource check(String urlId, GroupControl ctl) + private static GroupResource check(String id, GroupControl ctl) throws ResourceNotFoundException { if (ctl.isVisible()) { return new GroupResource(ctl); } - throw new ResourceNotFoundException(urlId); + throw new ResourceNotFoundException(id); } @SuppressWarnings("unchecked") @Override - public CreateGroup create(TopLevelResource root, String name) { - return createGroup.create(Url.decode(name)); + public CreateGroup create(TopLevelResource root, IdString name) { + return createGroup.create(name.get()); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java index 0243bd4fe8..fb5d13cc3a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java @@ -18,9 +18,9 @@ import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsCreate; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; -import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.group.AddIncludedGroups.PutIncludedGroup; @@ -55,14 +55,14 @@ public class IncludedGroupsCollection implements } @Override - public IncludedGroupResource parse(GroupResource parent, String id) + public IncludedGroupResource parse(GroupResource parent, IdString id) throws ResourceNotFoundException, OrmException { if (!parent.isInternal()) { throw new ResourceNotFoundException(id); } GroupDescription.Internal p = (GroupDescription.Internal) parent.getGroup(); - GroupResource included = groupsCollection.get().parse(id); + GroupResource included = groupsCollection.get().parse(id.get()); AccountGroupIncludeByUuid in = dbProvider.get() .accountGroupIncludesByUuid().get( new AccountGroupIncludeByUuid.Key( @@ -76,8 +76,8 @@ public class IncludedGroupsCollection implements @SuppressWarnings("unchecked") @Override - public PutIncludedGroup create(final GroupResource group, final String id) { - return new PutIncludedGroup(put, Url.decode(id)); + public PutIncludedGroup create(GroupResource group, IdString id) { + return new PutIncludedGroup(put, id.get()); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java index d32bd6eb65..24d7c3126e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java @@ -18,6 +18,7 @@ import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsCreate; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.Url; @@ -68,9 +69,9 @@ public class MembersCollection implements } @Override - public MemberResource parse(final GroupResource parent, final String id) + public MemberResource parse(GroupResource parent, IdString id) throws ResourceNotFoundException, Exception { - final Account a = accountResolver.find(Url.decode(id)); + Account a = accountResolver.find(id.get()); if (a == null) { throw new ResourceNotFoundException(id); } @@ -89,8 +90,8 @@ public class MembersCollection implements @SuppressWarnings("unchecked") @Override - public PutMember create(final GroupResource group, final String id) { - return new PutMember(put, Url.decode(id)); + public PutMember create(GroupResource group, IdString id) { + return new PutMember(put, id.get()); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java index 02aca3d496..6d30b4223d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginsCollection.java @@ -16,11 +16,11 @@ package com.google.gerrit.server.plugins; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsCreate; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.TopLevelResource; -import com.google.gerrit.extensions.restapi.Url; import com.google.inject.Inject; import com.google.inject.Provider; @@ -46,9 +46,9 @@ public class PluginsCollection implements } @Override - public PluginResource parse(TopLevelResource parent, String id) - throws ResourceNotFoundException, Exception { - Plugin p = loader.get(Url.decode(id)); + public PluginResource parse(TopLevelResource parent, IdString id) + throws ResourceNotFoundException { + Plugin p = loader.get(id.get()); if (p == null) { throw new ResourceNotFoundException(id); } @@ -57,9 +57,9 @@ public class PluginsCollection implements @SuppressWarnings("unchecked") @Override - public InstallPlugin create(TopLevelResource parent, String id) + public InstallPlugin create(TopLevelResource parent, IdString id) throws ResourceNotFoundException { - return new InstallPlugin(loader, Url.decode(id), true /* created */); + return new InstallPlugin(loader, id.get(), true /* created */); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java index 32b27c7701..38690d1022 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DashboardsCollection.java @@ -24,6 +24,7 @@ import com.google.common.collect.Lists; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.AcceptsCreate; import com.google.gerrit.extensions.restapi.ChildCollection; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestModifyView; @@ -77,30 +78,30 @@ class DashboardsCollection implements @SuppressWarnings("unchecked") @Override public RestModifyView create(ProjectResource parent, - String id) throws RestApiException { - if ("default".equals(id)) { + IdString id) throws RestApiException { + if (id.equals("default")) { return createDefault.get(); } throw new ResourceNotFoundException(id); } @Override - public DashboardResource parse(ProjectResource parent, String id) + public DashboardResource parse(ProjectResource parent, IdString id) throws ResourceNotFoundException, IOException, ConfigInvalidException { ProjectControl myCtl = parent.getControl(); - if ("default".equals(id)) { + if (id.equals("default")) { return DashboardResource.projectDefault(myCtl); } List parts = Lists.newArrayList( - Splitter.on(':').limit(2).split(id)); + Splitter.on(':').limit(2).split(id.get())); if (parts.size() != 2) { throw new ResourceNotFoundException(id); } CurrentUser user = myCtl.getCurrentUser(); - String ref = Url.decode(parts.get(0)); - String path = Url.decode(parts.get(1)); + String ref = parts.get(0); + String path = parts.get(1); for (ProjectState ps : myCtl.getProjectState().tree()) { try { return parse(ps.controlFor(user), ref, path, myCtl); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetDashboard.java index 72b9de611d..51d61910a1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetDashboard.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetDashboard.java @@ -16,9 +16,13 @@ package com.google.gerrit.server.project; import static com.google.gerrit.server.git.GitRepositoryManager.REFS_DASHBOARDS; +import com.google.common.base.Splitter; import com.google.common.base.Strings; +import com.google.common.collect.Lists; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.server.project.DashboardsCollection.DashboardInfo; import com.google.inject.Inject; @@ -26,6 +30,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException; import org.kohsuke.args4j.Option; import java.io.IOException; +import java.util.List; class GetDashboard implements RestReadView { private final DashboardsCollection dashboards; @@ -70,7 +75,7 @@ class GetDashboard implements RestReadView { if ("default".equals(id)) { throw new ResourceNotFoundException(); } else if (!Strings.isNullOrEmpty(id)) { - return dashboards.parse(new ProjectResource(ctl), id); + return parse(ctl, id); } else if (!inherited) { throw new ResourceNotFoundException(); } @@ -81,9 +86,19 @@ class GetDashboard implements RestReadView { throw new ResourceNotFoundException(); } else if (!Strings.isNullOrEmpty(id)) { ctl = ps.controlFor(ctl.getCurrentUser()); - return dashboards.parse(new ProjectResource(ctl), id); + return parse(ctl, id); } } throw new ResourceNotFoundException(); } + + private DashboardResource parse(ProjectControl ctl, String id) + throws ResourceNotFoundException, IOException, ConfigInvalidException { + List p = Lists.newArrayList(Splitter.on(':').limit(2).split(id)); + String ref = Url.encode(p.get(0)); + String path = Url.encode(p.get(1)); + return dashboards.parse( + new ProjectResource(ctl), + IdString.fromUrl(ref + ':' + path)); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java index 0883d24e81..68e6941768 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java @@ -15,11 +15,11 @@ package com.google.gerrit.server.project; import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.TopLevelResource; -import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.OutputFormat; @@ -50,12 +50,12 @@ public class ProjectsCollection implements } @Override - public ProjectResource parse(TopLevelResource parent, String id) + public ProjectResource parse(TopLevelResource parent, IdString id) throws ResourceNotFoundException { ProjectControl ctl; try { ctl = controlFactory.controlFor( - new Project.NameKey(Url.decode(id)), + new Project.NameKey(id.get()), user.get()); } catch (NoSuchProjectException e) { throw new ResourceNotFoundException(id); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java index 4050f4cfcc..3f70bc206d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java @@ -18,9 +18,10 @@ import com.google.common.base.Objects; import com.google.common.base.Strings; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; -import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.IdentifiedUser; @@ -73,7 +74,9 @@ class SetDefaultDashboard implements RestModifyView { DashboardResource target = null; if (input.id != null) { try { - target = dashboards.parse(new ProjectResource(ctl), input.id); + target = dashboards.parse( + new ProjectResource(ctl), + IdString.fromUrl(input.id)); } catch (ResourceNotFoundException e) { throw new BadRequestException("dashboard " + input.id + " not found"); }