From 6652a039dd373c17a62495ce80fac72b34495534 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Fri, 27 Nov 2020 12:40:02 +0100 Subject: [PATCH] fix unit tests --- .../java/sonia/scm/repository/GitUtil.java | 347 ++++++------------ .../repository/spi/GitTagsCommandTest.java | 19 +- scm-ui/ui-types/src/Changesets.ts | 6 +- scm-ui/ui-types/src/index.ts | 13 +- 4 files changed, 145 insertions(+), 240 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java index f41628f8e0..5650f59938 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java @@ -30,7 +30,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Multimap; -import org.checkerframework.checker.nullness.Opt; import org.eclipse.jgit.api.FetchCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; @@ -53,7 +52,6 @@ import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.LfsFactory; -import org.eclipse.jgit.util.RawParseUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.ContextEntry; @@ -64,11 +62,9 @@ import sonia.scm.util.Util; import sonia.scm.web.GitUserAgentProvider; import javax.servlet.http.HttpServletRequest; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.util.Arrays; import java.util.Collections; import java.util.Map; import java.util.Optional; @@ -80,73 +76,89 @@ import static java.util.Optional.of; //~--- JDK imports ------------------------------------------------------------ /** - * * @author Sebastian Sdorra */ -public final class GitUtil -{ +public final class GitUtil { private static final GitUserAgentProvider GIT_USER_AGENT_PROVIDER = new GitUserAgentProvider(); - /** Field description */ + /** + * Field description + */ public static final String REF_HEAD = "HEAD"; - /** Field description */ + /** + * Field description + */ public static final String REF_HEAD_PREFIX = "refs/heads/"; - /** Field description */ + /** + * Field description + */ public static final String REF_MASTER = "master"; - /** Field description */ + /** + * Field description + */ private static final String DIRECTORY_DOTGIT = ".git"; - /** Field description */ + /** + * Field description + */ private static final String DIRECTORY_OBJETCS = "objects"; - /** Field description */ + /** + * Field description + */ private static final String DIRECTORY_REFS = "refs"; - /** Field description */ + /** + * Field description + */ private static final String PREFIX_HEADS = "refs/heads/"; - /** Field description */ + /** + * Field description + */ private static final String PREFIX_TAG = "refs/tags/"; - /** Field description */ + /** + * Field description + */ private static final String REFSPEC = "+refs/heads/*:refs/remote/scm/%s/*"; - /** Field description */ + /** + * Field description + */ private static final String REMOTE_REF = "refs/remote/scm/%s/%s"; - /** Field description */ + /** + * Field description + */ private static final int TIMEOUT = 5; - /** Field description */ - private static final String USERAGENT_GIT = "git/"; - - /** the logger for GitUtil */ + /** + * the logger for GitUtil + */ private static final Logger logger = LoggerFactory.getLogger(GitUtil.class); //~--- constructors --------------------------------------------------------- /** * Constructs ... - * */ - private GitUtil() {} + private GitUtil() { + } //~--- methods -------------------------------------------------------------- /** * Method description * - * * @param repo */ - public static void close(org.eclipse.jgit.lib.Repository repo) - { - if (repo != null) - { + public static void close(org.eclipse.jgit.lib.Repository repo) { + if (repo != null) { repo.close(); } } @@ -154,42 +166,30 @@ public final class GitUtil /** * TODO cache * - * * @param repository * @param revWalk - * - * * @return */ public static Multimap createTagMap(org.eclipse.jgit.lib.Repository repository, - RevWalk revWalk) - { + RevWalk revWalk) { Multimap tags = ArrayListMultimap.create(); Map tagMap = repository.getTags(); - if (tagMap != null) - { - for (Map.Entry e : tagMap.entrySet()) - { - try - { + if (tagMap != null) { + for (Map.Entry e : tagMap.entrySet()) { + try { RevCommit c = getCommit(repository, revWalk, e.getValue()); - if (c != null) - { + if (c != null) { tags.put(c.getId(), e.getKey()); - } - else if (logger.isWarnEnabled()) - { + } else if (logger.isWarnEnabled()) { logger.warn("could not find commit for tag {}", e.getKey()); } - } - catch (IOException ex) - { + } catch (IOException ex) { logger.error("could not read commit for ref", ex); } @@ -200,8 +200,7 @@ public final class GitUtil } public static FetchResult fetch(Git git, File directory, Repository remoteRepository) { - try - { + try { FetchCommand fetch = git.fetch(); fetch.setRemote(directory.getAbsolutePath()); @@ -209,9 +208,7 @@ public final class GitUtil fetch.setTimeout((int) TimeUnit.MINUTES.toSeconds(TIMEOUT)); return fetch.call(); - } - catch (GitAPIException ex) - { + } catch (GitAPIException ex) { throw new InternalRepositoryException(ContextEntry.ContextBuilder.entity("Remote", directory.toString()).in(remoteRepository), "could not fetch", ex); } } @@ -219,29 +216,22 @@ public final class GitUtil /** * Method description * - * * @param directory - * * @return - * * @throws IOException */ public static org.eclipse.jgit.lib.Repository open(File directory) - throws IOException - { + throws IOException { FS fs = FS.DETECTED; FileRepositoryBuilder builder = new FileRepositoryBuilder(); builder.setFS(fs); - if (isGitDirectory(fs, directory)) - { + if (isGitDirectory(fs, directory)) { // bare repository builder.setGitDir(directory).setBare(); - } - else - { + } else { builder.setWorkTree(directory); } @@ -251,13 +241,10 @@ public final class GitUtil /** * Method description * - * * @param formatter */ - public static void release(DiffFormatter formatter) - { - if (formatter != null) - { + public static void release(DiffFormatter formatter) { + if (formatter != null) { formatter.close(); } } @@ -265,13 +252,10 @@ public final class GitUtil /** * Method description * - * * @param walk */ - public static void release(TreeWalk walk) - { - if (walk != null) - { + public static void release(TreeWalk walk) { + if (walk != null) { walk.close(); } } @@ -279,13 +263,10 @@ public final class GitUtil /** * Method description * - * * @param walk */ - public static void release(RevWalk walk) - { - if (walk != null) - { + public static void release(RevWalk walk) { + if (walk != null) { walk.close(); } } @@ -295,17 +276,13 @@ public final class GitUtil /** * Method description * - * * @param ref - * * @return */ - public static String getBranch(Ref ref) - { + public static String getBranch(Ref ref) { String branch = null; - if (ref != null) - { + if (ref != null) { branch = getBranch(ref.getName()); } @@ -315,17 +292,13 @@ public final class GitUtil /** * Method description * - * * @param name - * * @return */ - public static String getBranch(String name) - { + public static String getBranch(String name) { String branch = null; - if (Util.isNotEmpty(name) && name.startsWith(PREFIX_HEADS)) - { + if (Util.isNotEmpty(name) && name.startsWith(PREFIX_HEADS)) { branch = name.substring(PREFIX_HEADS.length()); } @@ -336,18 +309,15 @@ public final class GitUtil * Returns {@code true} if the provided reference name is a branch name. * * @param refName reference name - * * @return {@code true} if the name is a branch name - * * @since 1.50 */ - public static boolean isBranch(String refName) - { + public static boolean isBranch(String refName) { return Strings.nullToEmpty(refName).startsWith(PREFIX_HEADS); } public static Ref getBranchIdOrCurrentHead(org.eclipse.jgit.lib.Repository gitRepository, String requestedBranch) throws IOException { - if ( Strings.isNullOrEmpty(requestedBranch) ) { + if (Strings.isNullOrEmpty(requestedBranch)) { logger.trace("no default branch configured, use repository head as default"); Optional repositoryHeadRef = GitUtil.getRepositoryHeadRef(gitRepository); return repositoryHeadRef.orElse(null); @@ -359,37 +329,28 @@ public final class GitUtil /** * Method description * - * * @param repo * @param branchName - * * @return - * * @throws IOException */ public static Ref getBranchId(org.eclipse.jgit.lib.Repository repo, - String branchName) - throws IOException - { + String branchName) + throws IOException { Ref ref = null; - if (!branchName.startsWith(REF_HEAD)) - { + if (!branchName.startsWith(REF_HEAD)) { branchName = PREFIX_HEADS.concat(branchName); } checkBranchName(repo, branchName); - try - { + try { ref = repo.findRef(branchName); - if (ref == null) - { + if (ref == null) { logger.warn("could not find branch for {}", branchName); } - } - catch (IOException ex) - { + } catch (IOException ex) { logger.warn("error occured during resolve of branch id", ex); } @@ -426,21 +387,17 @@ public final class GitUtil * If the given ref is for a tag, the commit that this tag belongs to is returned instead. */ public static RevCommit getCommit(org.eclipse.jgit.lib.Repository repository, - RevWalk revWalk, Ref ref) - throws IOException - { + RevWalk revWalk, Ref ref) + throws IOException { RevCommit commit = null; ObjectId id = ref.getPeeledObjectId(); - if (id == null) - { + if (id == null) { id = ref.getObjectId(); } - if (id != null) - { - if (revWalk == null) - { + if (id != null) { + if (revWalk == null) { revWalk = new RevWalk(repository); } @@ -451,16 +408,13 @@ public final class GitUtil } public static RevTag getTag(org.eclipse.jgit.lib.Repository repository, - RevWalk revWalk, Ref ref) - throws IOException - { + RevWalk revWalk, Ref ref) + throws IOException { RevTag tag = null; ObjectId id = ref.getObjectId(); - if (id != null) - { - if (revWalk == null) - { + if (id != null) { + if (revWalk == null) { revWalk = new RevWalk(repository); } @@ -473,13 +427,10 @@ public final class GitUtil /** * Method description * - * * @param commit - * * @return */ - public static long getCommitTime(RevCommit commit) - { + public static long getCommitTime(RevCommit commit) { long date = commit.getCommitTime(); date = date * 1000; @@ -490,17 +441,13 @@ public final class GitUtil /** * Method description * - * * @param objectId - * * @return */ - public static String getId(AnyObjectId objectId) - { + public static String getId(AnyObjectId objectId) { String id = Util.EMPTY_STRING; - if (objectId != null) - { + if (objectId != null) { id = objectId.name(); } @@ -510,44 +457,27 @@ public final class GitUtil /** * Method description * - * * @param repository * @param id - * * @return - * * @throws IOException */ public static Ref getRefForCommit(org.eclipse.jgit.lib.Repository repository, - ObjectId id) - throws IOException - { + ObjectId id) + throws IOException { Ref ref = null; - RevWalk walk = null; - - try - { - walk = new RevWalk(repository); + try (RevWalk walk = new RevWalk(repository)) { RevCommit commit = walk.parseCommit(id); - for (Map.Entry e : repository.getAllRefs().entrySet()) - { - if (e.getKey().startsWith(Constants.R_HEADS)) - { - if (walk.isMergedInto(commit, - walk.parseCommit(e.getValue().getObjectId()))) - { - ref = e.getValue(); - } + for (Map.Entry e : repository.getAllRefs().entrySet()) { + if (e.getKey().startsWith(Constants.R_HEADS) && walk.isMergedInto(commit, + walk.parseCommit(e.getValue().getObjectId()))) { + ref = e.getValue(); } } } - finally - { - release(walk); - } return ref; } @@ -599,26 +529,19 @@ public final class GitUtil /** * Method description * - * * @param repo * @param revision - * * @return - * * @throws IOException */ public static ObjectId getRevisionId(org.eclipse.jgit.lib.Repository repo, - String revision) - throws IOException - { + String revision) + throws IOException { ObjectId revId; - if (Util.isNotEmpty(revision)) - { + if (Util.isNotEmpty(revision)) { revId = repo.resolve(revision); - } - else - { + } else { revId = getRepositoryHead(repo); } @@ -628,34 +551,27 @@ public final class GitUtil /** * Method description * - * * @param repository * @param localBranch - * * @return */ public static String getScmRemoteRefName(Repository repository, - Ref localBranch) - { + Ref localBranch) { return getScmRemoteRefName(repository, localBranch.getName()); } /** * Method description * - * * @param repository * @param localBranch - * * @return */ public static String getScmRemoteRefName(Repository repository, - String localBranch) - { + String localBranch) { String branch = localBranch; - if (localBranch.startsWith(REF_HEAD_PREFIX)) - { + if (localBranch.startsWith(REF_HEAD_PREFIX)) { branch = localBranch.substring(REF_HEAD_PREFIX.length()); } @@ -666,16 +582,12 @@ public final class GitUtil * Returns the name of the tag or {@code null} if the the ref is not a tag. * * @param refName ref name - * * @return name of tag or {@link null} - * * @since 1.50 */ - public static String getTagName(String refName) - { + public static String getTagName(String refName) { String tagName = null; - if (refName.startsWith(PREFIX_TAG)) - { + if (refName.startsWith(PREFIX_TAG)) { tagName = refName.substring(PREFIX_TAG.length()); } @@ -685,17 +597,13 @@ public final class GitUtil /** * Method description * - * * @param ref - * * @return */ - public static String getTagName(Ref ref) - { + public static String getTagName(Ref ref) { String name = ref.getName(); - if (name.startsWith(PREFIX_TAG)) - { + if (name.startsWith(PREFIX_TAG)) { name = name.substring(PREFIX_TAG.length()); } @@ -706,8 +614,8 @@ public final class GitUtil public static Optional getTagSignature(RevObject revObject, GPG gpg, RevWalk revWalk) throws IOException { if (revObject instanceof RevTag) { - final byte[] bytes = revWalk.getObjectReader().open(revObject.getId()).getBytes(); - final String message = new String(bytes); + final byte[] messageBytes = revWalk.getObjectReader().open(revObject.getId()).getBytes(); + final String message = new String(messageBytes); final int signatureStartIndex = message.indexOf(GPG_HEADER); if (signatureStartIndex < 0) { return Optional.empty(); @@ -729,7 +637,7 @@ public final class GitUtil PublicKey publicKey = publicKeyById.get(); - boolean verified = publicKey.verify(message.substring(0, signatureStartIndex - 1).getBytes(), signature.getBytes()); + boolean verified = publicKey.verify(messageBytes, signature.getBytes()); return Optional.of(new Signature( publicKeyId, "gpg", @@ -744,71 +652,56 @@ public final class GitUtil /** * Returns true if the request comes from a git client. * - * * @param request servlet request - * * @return true if the client is git */ - public static boolean isGitClient(HttpServletRequest request) - { + public static boolean isGitClient(HttpServletRequest request) { return GIT_USER_AGENT_PROVIDER.parseUserAgent(request.getHeader(HttpUtil.HEADER_USERAGENT)) != null; } /** * Method description * - * * @param dir - * * @return */ - public static boolean isGitDirectory(File dir) - { + public static boolean isGitDirectory(File dir) { return isGitDirectory(FS.DETECTED, dir); } /** * Method description * - * * @param fs * @param dir - * * @return */ - public static boolean isGitDirectory(FS fs, File dir) - { + public static boolean isGitDirectory(FS fs, File dir) { //J- return fs.resolve(dir, DIRECTORY_OBJETCS).exists() && fs.resolve(dir, DIRECTORY_REFS).exists() - &&!fs.resolve(dir, DIRECTORY_DOTGIT).exists(); + && !fs.resolve(dir, DIRECTORY_DOTGIT).exists(); //J+ } /** * Method description * - * * @param ref - * * @return */ - public static boolean isHead(String ref) - { + public static boolean isHead(String ref) { return ref.startsWith(REF_HEAD_PREFIX); } /** * Method description * - * * @param id - * * @return */ - public static boolean isValidObjectId(ObjectId id) - { - return (id != null) &&!id.equals(ObjectId.zeroId()); + public static boolean isValidObjectId(ObjectId id) { + return (id != null) && !id.equals(ObjectId.zeroId()); } /** @@ -850,25 +743,20 @@ public final class GitUtil /** * Method description * - * * @param repo * @param branchName - * * @throws IOException */ @VisibleForTesting static void checkBranchName(org.eclipse.jgit.lib.Repository repo, - String branchName) - throws IOException - { - if (branchName.contains("..")) - { + String branchName) + throws IOException { + if (branchName.contains("..")) { File repoDirectory = repo.getDirectory(); File branchFile = new File(repoDirectory, branchName); if (!branchFile.getCanonicalPath().startsWith( - repoDirectory.getCanonicalPath())) - { + repoDirectory.getCanonicalPath())) { logger.error( "branch \"{}\" is outside of the repository. It looks like path traversal attack", branchName); @@ -882,13 +770,10 @@ public final class GitUtil /** * Method description * - * * @param repository - * * @return */ - private static RefSpec createRefSpec(Repository repository) - { + private static RefSpec createRefSpec(Repository repository) { return new RefSpec(String.format(REFSPEC, repository.getId())); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitTagsCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitTagsCommandTest.java index 456096d736..f93fd801c1 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitTagsCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitTagsCommandTest.java @@ -84,10 +84,23 @@ public class GitTagsCommandTest extends AbstractGitCommandTestBase { "1vSkcjK26RqhAqCjNLSagM8ATZrh+g==\n" + "=kUKm\n" + "-----END PGP SIGNATURE-----\n"; - String signedContent = "Tagger: Arthur Dent \n" + - "Date: Tue Nov 24 21:37:46 2020 +0100\n" + + String signedContent = "object 592d797cd36432e591416e8b2b98154f4f163411\n" + + "type commit\n" + + "tag signedtag\n" + + "tagger Arthur Dent 1606248906 +0100\n" + "\n" + - "this tag is signed"; + "this tag is signed\n" + + "-----BEGIN PGP SIGNATURE-----\n" + + "\n" + + "iQEzBAABCgAdFiEEK6J3IfETwAXMFvBrrmPvvEnxQM8FAl+9acoACgkQrmPvvEnx\n" + + "QM9abwgAnGP+Y/Ijli+PAsimfOmZQWYepjptoOv9m7i3bnHv8V+Qg6cm51I3E0YV\n" + + "R2QaxxzW9PgS4hcES+L1qs8Lwo18RurF469eZEmNb8DcUFJ3sEWeHlIl5wZNNo/v\n" + + "jJm0d9LNcSmtAIiQ8eDMoGdFXJzHewGickLOSsQGmfZgZus4Qlsh7r3BZTI1Zwd/\n" + + "6jaBFctX13FuepCTxq2SjEfRaQHIYkyFQq2o6mjL5S2qfYJ/S//gcCCzxllQrisF\n" + + "5fRW3LzLI4eXFH0vua7+UzNS2Rwpifg2OENJA/Kn+3R36LWEGxFK9pNqjVPRAcQj\n" + + "1vSkcjK26RqhAqCjNLSagM8ATZrh+g==\n" + + "=kUKm\n" + + "-----END PGP SIGNATURE-----\n"; when(publicKey.verify(signedContent.getBytes(), signature.getBytes())).thenReturn(true); final GitContext gitContext = createContext(); diff --git a/scm-ui/ui-types/src/Changesets.ts b/scm-ui/ui-types/src/Changesets.ts index 561a8c5045..ab7fa54e61 100644 --- a/scm-ui/ui-types/src/Changesets.ts +++ b/scm-ui/ui-types/src/Changesets.ts @@ -22,11 +22,11 @@ * SOFTWARE. */ -import {Collection, Link, Links} from "./hal"; +import { Collection, Links } from "./hal"; import { Tag } from "./Tags"; import { Branch } from "./Branches"; import { Person } from "./Person"; -import {Signature} from "./Signature"; +import { Signature } from "./Signature"; export type Changeset = Collection & { id: string; @@ -43,8 +43,6 @@ export type Changeset = Collection & { }; }; - - export type Contributor = { person: Person; type: string; diff --git a/scm-ui/ui-types/src/index.ts b/scm-ui/ui-types/src/index.ts index 35adda77bb..da97204e1a 100644 --- a/scm-ui/ui-types/src/index.ts +++ b/scm-ui/ui-types/src/index.ts @@ -29,14 +29,23 @@ export { Me } from "./Me"; export { DisplayedUser, User } from "./User"; export { Group, Member } from "./Group"; -export { Repository, RepositoryCollection, RepositoryGroup, RepositoryCreation, Namespace, NamespaceCollection } from "./Repositories"; +export { + Repository, + RepositoryCollection, + RepositoryGroup, + RepositoryCreation, + Namespace, + NamespaceCollection +} from "./Repositories"; export { RepositoryType, RepositoryTypeCollection } from "./RepositoryTypes"; export { Branch, BranchRequest } from "./Branches"; export { Person } from "./Person"; -export { Changeset, Contributor, ParentChangeset, Signature } from "./Changesets"; +export { Changeset, Contributor, ParentChangeset } from "./Changesets"; + +export { Signature } from "./Signature"; export { AnnotatedSource, AnnotatedLine } from "./Annotate";