From 62460b42b7a64513e3421aae17082fdc923faf95 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 8 Jun 2018 17:45:00 +0200 Subject: [PATCH 1/5] Validate branch names on branch creation Since v2.16.0-rc0~89^2~1 (branch: correctly reject refs/heads/{-dash,HEAD}, 2017-11-14), native git does not allow branch names - refs/heads/HEAD - starting with '-' Bug: 535655 Change-Id: Ib1c4ec9ea844073901a4ebe6a29ff6cc8ae58e93 Signed-off-by: Matthias Sohn --- .../eclipse/jgit/api/BranchCommandTest.java | 12 +++++++ .../eclipse/jgit/api/CreateBranchCommand.java | 32 +++++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BranchCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BranchCommandTest.java index 2fe40b99e..08f1fdfac 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BranchCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/BranchCommandTest.java @@ -191,6 +191,18 @@ public class BranchCommandTest extends RepositoryTestCase { - allBefore); } + @Test(expected = InvalidRefNameException.class) + public void testInvalidBranchHEAD() throws Exception { + git.branchCreate().setName("HEAD").call(); + fail("Create branch with invalid ref name should fail"); + } + + @Test(expected = InvalidRefNameException.class) + public void testInvalidBranchDash() throws Exception { + git.branchCreate().setName("-x").call(); + fail("Create branch with invalid ref name should fail"); + } + @Test public void testListAllBranchesShouldNotDie() throws Exception { setUpRepoWithRemote().branchList().setListMode(ListMode.ALL).call(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CreateBranchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CreateBranchCommand.java index 29baf4cd6..ba6f3f11b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CreateBranchCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CreateBranchCommand.java @@ -43,6 +43,9 @@ */ package org.eclipse.jgit.api; +import static org.eclipse.jgit.lib.Constants.HEAD; +import static org.eclipse.jgit.lib.Constants.R_HEADS; + import java.io.IOException; import java.text.MessageFormat; @@ -78,7 +81,7 @@ public class CreateBranchCommand extends GitCommand { private SetupUpstreamMode upstreamMode; - private String startPoint = Constants.HEAD; + private String startPoint = HEAD; private RevCommit startCommit; @@ -121,7 +124,7 @@ public class CreateBranchCommand extends GitCommand { try (RevWalk revWalk = new RevWalk(repo)) { Ref refToCheck = repo.findRef(name); boolean exists = refToCheck != null - && refToCheck.getName().startsWith(Constants.R_HEADS); + && refToCheck.getName().startsWith(R_HEADS); if (!force && exists) throw new RefAlreadyExistsException(MessageFormat.format( JGitText.get().refAlreadyExists1, name)); @@ -153,7 +156,7 @@ public class CreateBranchCommand extends GitCommand { else refLogMessage = "branch: Created from commit " + baseCommit; //$NON-NLS-1$ - } else if (startPointFullName.startsWith(Constants.R_HEADS) + } else if (startPointFullName.startsWith(R_HEADS) || startPointFullName.startsWith(Constants.R_REMOTES)) { baseBranch = startPointFullName; if (exists) @@ -171,7 +174,7 @@ public class CreateBranchCommand extends GitCommand { + startPointFullName; } - RefUpdate updateRef = repo.updateRef(Constants.R_HEADS + name); + RefUpdate updateRef = repo.updateRef(R_HEADS + name); updateRef.setNewObjectId(startAt); updateRef.setRefLogMessage(refLogMessage, false); Result updateResult; @@ -279,16 +282,33 @@ public class CreateBranchCommand extends GitCommand { } private String getStartPointOrHead() { - return startPoint != null ? startPoint : Constants.HEAD; + return startPoint != null ? startPoint : HEAD; } private void processOptions() throws InvalidRefNameException { if (name == null - || !Repository.isValidRefName(Constants.R_HEADS + name)) + || !Repository.isValidRefName(R_HEADS + name) + || !isValidBranchName(name)) throw new InvalidRefNameException(MessageFormat.format(JGitText .get().branchNameInvalid, name == null ? "" : name)); //$NON-NLS-1$ } + /** + * Check if the given branch name is valid + * + * @param branchName + * branch name to check + * @return {@code true} if the branch name is valid + * + * @since 5.0 + */ + public static boolean isValidBranchName(String branchName) { + if (HEAD.equals(branchName)) { + return false; + } + return !branchName.startsWith("-"); //$NON-NLS-1$ + } + /** * Set the name of the new branch * From 5fe8e31d4351a9d26db81e799defd8225e883f3e Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 5 Jan 2018 13:02:47 -0500 Subject: [PATCH 2/5] Ensure DirectoryStream is closed promptly From the javadoc for Files.list: "The returned stream encapsulates a DirectoryStream. If timely disposal of file system resources is required, the try-with-resources construct should be used to ensure that the stream's close method is invoked after the stream operations are completed." This is the only call to Files#newDirectoryStream that is not already in a try-with-resources. Change-Id: I91e6c56b5d74e8435457ad6ed9e6b4b24d2aa14e (cherry picked from commit 1c16ea4601920c9dbc7a0202efc20137e1a63d55) --- .../jgit/internal/storage/file/GC.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 928e52d6b..60d665274 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -966,19 +966,19 @@ public class GC { Path packDir = Paths.get(repo.getObjectsDirectory().getAbsolutePath(), "pack"); //$NON-NLS-1$ Instant threshold = Instant.now().minus(1, ChronoUnit.DAYS); - try { - Files.newDirectoryStream(packDir, "gc_*_tmp") //$NON-NLS-1$ - .forEach(t -> { - try { - Instant lastModified = Files.getLastModifiedTime(t) - .toInstant(); - if (lastModified.isBefore(threshold)) { - Files.deleteIfExists(t); - } - } catch (IOException e) { - LOG.error(e.getMessage(), e); - } - }); + try (DirectoryStream stream = + Files.newDirectoryStream(packDir, "gc_*_tmp")) { //$NON-NLS-1$ + stream.forEach(t -> { + try { + Instant lastModified = Files.getLastModifiedTime(t) + .toInstant(); + if (lastModified.isBefore(threshold)) { + Files.deleteIfExists(t); + } + } catch (IOException e) { + LOG.error(e.getMessage(), e); + } + }); } catch (IOException e) { LOG.error(e.getMessage(), e); } From a9e6da108212332e7dd476342090b77f9a72034e Mon Sep 17 00:00:00 2001 From: Markus Duft Date: Thu, 12 Apr 2018 10:11:18 +0200 Subject: [PATCH 3/5] LFS: Better SSH authentication token timeout handling * Larger eager timeout to compensate for high-latency lines * Respect eager timeout in case the server uses "expiresIn" Change-Id: Id87da1eea874e70b69eaccf35c84af4c3bb50770 Signed-off-by: Markus Duft --- .../eclipse/jgit/lfs/internal/LfsConnectionFactory.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java index 25a70c745..3ac69923f 100644 --- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java +++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java @@ -273,7 +273,7 @@ public class LfsConnectionFactory { } private static final class AuthCache { - private static final long AUTH_CACHE_EAGER_TIMEOUT = 100; + private static final long AUTH_CACHE_EAGER_TIMEOUT = 500; private static final SimpleDateFormat ISO_FORMAT = new SimpleDateFormat( "yyyy-MM-dd'T'HH:mm:ss.SSSX"); //$NON-NLS-1$ @@ -290,8 +290,9 @@ public class LfsConnectionFactory { this.cachedAction = action; try { if (action.expiresIn != null && !action.expiresIn.isEmpty()) { - this.validUntil = System.currentTimeMillis() - + Long.parseLong(action.expiresIn); + this.validUntil = (System.currentTimeMillis() + + Long.parseLong(action.expiresIn)) + - AUTH_CACHE_EAGER_TIMEOUT; } else if (action.expiresAt != null && !action.expiresAt.isEmpty()) { this.validUntil = ISO_FORMAT.parse(action.expiresAt) From 5f8b6ebc9f6b3da733ff5b7ec8497312cd4d50e2 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 9 Jun 2018 02:01:59 +0200 Subject: [PATCH 4/5] RawTextTest#testBinary: use array comparison to compare arrays Change-Id: Iac1feadf24858a0bdf0cb224f16b34e9498fe3bb Signed-off-by: Matthias Sohn --- .../tst/org/eclipse/jgit/diff/RawTextTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java index 8cf3eedfd..58a8b2d46 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RawTextTest.java @@ -45,6 +45,7 @@ package org.eclipse.jgit.diff; import static org.eclipse.jgit.lib.Constants.CHARSET; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -69,7 +70,7 @@ public class RawTextTest { String input = "foo-a\nf\0o-b\n"; byte[] data = Constants.encodeASCII(input); final RawText a = new RawText(data); - assertEquals(a.content, data); + assertArrayEquals(a.content, data); assertEquals(a.size(), 1); assertEquals(a.getString(0, 1, false), input); } From 4ef8769f81949d1b5759645bdba969b6b5a7289a Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 8 Jun 2018 15:47:05 +0200 Subject: [PATCH 5/5] Ensure Jsch checks all configured algorithms Jsch checks only for the availability of the algorithms given by Jsch-internal config keys "CheckCiphers", "CheckKexes", and "CheckSignatures". If the ssh config defines any algorithms unknown to Jsch not listed in those keys, it'll still propose them during the negotiation phase, and run into an NPE later on if the server happens to propose such an algorithm and it gets chosen. Jsch reads those "CheckCiphers" and the other values from either a session-local config, or the global static Jsch config. It bypasses ~/.ssh/config for these values. Therefore, copy these values from the config as read from ~/.ssh/config into the session-specific config. That makes Jsch check _all_ configured algorithms up front, discarding any for which it has no implementation. Thus it proposes only algorithms it actually can handle. Bug: 535672 Change-Id: I6a68e54f4d9a3267e895c536bcf3c58099826ad5 Signed-off-by: Thomas Wolf --- .../transport/JschConfigSessionFactory.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java index ea2f4b1e3..1d5248a15 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java @@ -70,6 +70,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.jcraft.jsch.ConfigRepository; +import com.jcraft.jsch.ConfigRepository.Config; import com.jcraft.jsch.JSch; import com.jcraft.jsch.JSchException; import com.jcraft.jsch.Session; @@ -222,10 +223,30 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory { session.setUserInfo(new CredentialsProviderUserInfo(session, credentialsProvider)); } + safeConfig(session, hc.getConfig()); configure(hc, session); return session; } + private void safeConfig(Session session, Config cfg) { + // Ensure that Jsch checks all configured algorithms, not just its + // built-in ones. Otherwise it may propose an algorithm for which it + // doesn't have an implementation, and then run into an NPE if that + // algorithm ends up being chosen. + copyConfigValueToSession(session, cfg, "Ciphers", "CheckCiphers"); //$NON-NLS-1$ //$NON-NLS-2$ + copyConfigValueToSession(session, cfg, "KexAlgorithms", "CheckKexes"); //$NON-NLS-1$ //$NON-NLS-2$ + copyConfigValueToSession(session, cfg, "HostKeyAlgorithms", //$NON-NLS-1$ + "CheckSignatures"); //$NON-NLS-1$ + } + + private void copyConfigValueToSession(Session session, Config cfg, + String from, String to) { + String value = cfg.getValue(from); + if (value != null) { + session.setConfig(to, value); + } + } + private void setUserName(Session session, String userName) { // Jsch 0.1.54 picks up the user name from the ssh config, even if an // explicit user name was given! We must correct that if ~/.ssh/config