motiejus
/
jgit
1
Fork 0
Code Releases Activity

Ensure that ~ in ssh config is replaced before Jsch sees it 

Do tilde replacement for values from the ssh config file that are
file names in all cases to make sure that they are already replaced
when Jsch tries to get the values.

Previously, OpenSshConfig did tilde replacement only for the
IdentityFile in the JGit-facing "Host" interface and left the
replacement in the Jsch-facing "Config" interface to Jsch.

But on Windows the JGit notion of what should be used to replace the
tilde differs from Jsch's replacement. Jsch always replaces the tilde
by the value of the system property "user.home", whereas JGit also
considers some environment variables like %HOME%. This can lead to
rather surprising failures as in the case of bug 526175 where
%HOME% != user.home.

Prior to commit 9d24470 (i.e.,prior to JGit 4.9.0) this problem never
occurred because Jsch was completely unaware of the ssh config file
and all host and IdentityFile handling happened exclusively in JGit.

Bug: 526175
Change-Id: I1511699664ffea07cb58ed751cfdb79b15e3a99e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This commit is contained in:
Thomas Wolf 2017-10-18 22:45:36 +02:00
parent 8180183289
commit adbf0935e1
2 changed files with 49 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
View File

@ -323,11 +323,12 @@ public void testListValueSingle() throws Exception {
@Test
public void testListValueMultiple() throws Exception {
// Tilde expansion doesn't occur within the parser
// Tilde expansion occurs within the parser
config("Host orcz\nUserKnownHostsFile \"~/foo/ba z\" /foo/bar \n");
final ConfigRepository.Config c = osc.getConfig("orcz");
assertNotNull(c);
assertArrayEquals(new Object[] { "~/foo/ba z", "/foo/bar" },
assertArrayEquals(new Object[] { new File(home, "foo/ba z").getPath(),
"/foo/bar" },
c.getValues("UserKnownHostsFile"));
}
@ -371,8 +372,9 @@ public void testIdentityFile() throws Exception {
// Host does tilde replacement
assertEquals(new File(home, "foo/ba z"), f);
final ConfigRepository.Config c = h.getConfig();
// Config doesn't
assertArrayEquals(new Object[] { "~/foo/ba z", "/foo/bar" },
// Config does tilde replacement, too
assertArrayEquals(new Object[] { new File(home, "foo/ba z").getPath(),
"/foo/bar" },
c.getValues("IdentityFile"));
}
@ -386,8 +388,9 @@ public void testMultiIdentityFile() throws Exception {
// Host does tilde replacement
assertEquals(new File(home, "foo/ba z"), f);
final ConfigRepository.Config c = h.getConfig();
// Config doesn't
assertArrayEquals(new Object[] { "~/foo/ba z", "/foo/bar", "/foo/baz" },
// Config does tilde replacement, too
assertArrayEquals(new Object[] { new File(home, "foo/ba z").getPath(),
"/foo/bar", "/foo/baz" },
c.getValues("IdentityFile"));
}
@ -397,7 +400,7 @@ public void testNegatedPattern() throws Exception {
final Host h = osc.lookup("repo.or.cz");
assertNotNull(h);
assertEquals(new File(home, "foo/bar"), h.getIdentityFile());
assertArrayEquals(new Object[] { "~/foo/bar" },
assertArrayEquals(new Object[] { new File(home, "foo/bar").getPath() },
h.getConfig().getValues("IdentityFile"));
}
@ -407,7 +410,8 @@ public void testPattern() throws Exception {
final Host h = osc.lookup("repo.or.cz");
assertNotNull(h);
assertEquals(new File(home, "foo/bar"), h.getIdentityFile());
assertArrayEquals(new Object[] { "~/foo/bar", "/foo/baz" },
assertArrayEquals(new Object[] { new File(home, "foo/bar").getPath(),
"/foo/baz" },
h.getConfig().getValues("IdentityFile"));
}
@ -417,12 +421,13 @@ public void testMultiHost() throws Exception {
final Host h1 = osc.lookup("repo.or.cz");
assertNotNull(h1);
assertEquals(new File(home, "foo/bar"), h1.getIdentityFile());
assertArrayEquals(new Object[] { "~/foo/bar", "/foo/baz" },
assertArrayEquals(new Object[] { new File(home, "foo/bar").getPath(),
"/foo/baz" },
h1.getConfig().getValues("IdentityFile"));
final Host h2 = osc.lookup("orcz");
assertNotNull(h2);
assertEquals(new File(home, "foo/bar"), h2.getIdentityFile());
assertArrayEquals(new Object[] { "~/foo/bar" },
assertArrayEquals(new Object[] { new File(home, "foo/bar").getPath() },
h2.getConfig().getValues("IdentityFile"));
}

45
org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java
View File

@ -352,6 +352,17 @@ private static Boolean yesno(final String value) {
return Boolean.FALSE;
}
private static File toFile(String path, File home) {
if (path.startsWith("~/")) { //$NON-NLS-1$
return new File(home, path.substring(2));
}
File ret = new File(path);
if (ret.isAbsolute()) {
return ret;
}
return new File(home, path);
}
private static int positive(final String value) {
if (value != null) {
try {
@ -730,25 +741,48 @@ private List<String> substitute(List<String> values, String allowed,
return result;
}
private List<String> replaceTilde(List<String> values, File home) {
List<String> result = new ArrayList<>(values.size());
for (String value : values) {
result.add(toFile(value, home).getPath());
}
return result;
}
protected void substitute(String originalHostName, File home) {
Replacer r = new Replacer(originalHostName, home);
if (multiOptions != null) {
List<String> values = multiOptions.get("IDENTITYFILE"); //$NON-NLS-1$
if (values != null) {
values = substitute(values, "dhlru", r); //$NON-NLS-1$
values = replaceTilde(values, home);
multiOptions.put("IDENTITYFILE", values); //$NON-NLS-1$
}
values = multiOptions.get("CERTIFICATEFILE"); //$NON-NLS-1$
if (values != null) {
values = substitute(values, "dhlru", r); //$NON-NLS-1$
values = replaceTilde(values, home);
multiOptions.put("CERTIFICATEFILE", values); //$NON-NLS-1$
}
}
if (listOptions != null) {
List<String> values = listOptions.get("GLOBALKNOWNHOSTSFILE"); //$NON-NLS-1$
if (values != null) {
values = replaceTilde(values, home);
listOptions.put("GLOBALKNOWNHOSTSFILE", values); //$NON-NLS-1$
}
values = listOptions.get("USERKNOWNHOSTSFILE"); //$NON-NLS-1$
if (values != null) {
values = replaceTilde(values, home);
listOptions.put("USERKNOWNHOSTSFILE", values); //$NON-NLS-1$
}
}
if (options != null) {
// HOSTNAME already done in Replacer constructor
String value = options.get("IDENTITYAGENT"); //$NON-NLS-1$
if (value != null) {
value = r.substitute(value, "dhlru"); //$NON-NLS-1$
value = toFile(value, home).getPath();
options.put("IDENTITYAGENT", value); //$NON-NLS-1$
}
}
@ -909,17 +943,6 @@ private void complete(String initialHostName, File homeDir) {
}
}
private File toFile(String path, File home) {
if (path.startsWith("~/")) { //$NON-NLS-1$
return new File(home, path.substring(2));
}
File ret = new File(path);
if (ret.isAbsolute()) {
return ret;
}
return new File(home, path);
}
Config getConfig() {
return config;
}