diff --git a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java index af09f499f..4c7e99ea8 100644 --- a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java +++ b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java @@ -467,4 +467,34 @@ public void testLocalhostFQDNReplacement() throws Exception { new File(new File(home, ".ssh"), localhost + "_id_dsa"), h.getIdentityFile()); } + + @Test + public void testPubKeyAcceptedAlgorithms() throws Exception { + config("Host=orcz\n\tPubkeyAcceptedAlgorithms ^ssh-rsa"); + Host h = osc.lookup("orcz"); + Config c = h.getConfig(); + assertEquals("^ssh-rsa", + c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS)); + assertEquals("^ssh-rsa", c.getValue("PubkeyAcceptedKeyTypes")); + } + + @Test + public void testPubKeyAcceptedKeyTypes() throws Exception { + config("Host=orcz\n\tPubkeyAcceptedKeyTypes ^ssh-rsa"); + Host h = osc.lookup("orcz"); + Config c = h.getConfig(); + assertEquals("^ssh-rsa", + c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS)); + assertEquals("^ssh-rsa", c.getValue("PubkeyAcceptedKeyTypes")); + } + + @Test + public void testEolComments() throws Exception { + config("#Comment\nHost=orcz #Comment\n\tPubkeyAcceptedAlgorithms ^ssh-rsa # Comment\n#Comment"); + Host h = osc.lookup("orcz"); + assertNotNull(h); + Config c = h.getConfig(); + assertEquals("^ssh-rsa", + c.getValue(SshConstants.PUBKEY_ACCEPTED_ALGORITHMS)); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java index 98c63cdcd..c514270f5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java @@ -23,7 +23,6 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TreeMap; @@ -224,8 +223,17 @@ private Map parse(BufferedReader reader) entries.put(DEFAULT_NAME, defaults); while ((line = reader.readLine()) != null) { + // OpenSsh ignores trailing comments on a line. Anything after the + // first # on a line is trimmed away (yes, even if the hash is + // inside quotes). + // + // See https://github.com/openssh/openssh-portable/commit/2bcbf679 + int i = line.indexOf('#'); + if (i >= 0) { + line = line.substring(0, i); + } line = line.trim(); - if (line.isEmpty() || line.startsWith("#")) { //$NON-NLS-1$ + if (line.isEmpty()) { continue; } String[] parts = line.split("[ \t]*[= \t]", 2); //$NON-NLS-1$ @@ -484,12 +492,30 @@ public static class HostEntry implements SshConfigStore.HostConfig { LIST_KEYS.add(SshConstants.USER_KNOWN_HOSTS_FILE); } + /** + * OpenSSH has renamed some config keys. This maps old names to new + * names. + */ + private static final Map ALIASES = new TreeMap<>( + String.CASE_INSENSITIVE_ORDER); + + static { + // See https://github.com/openssh/openssh-portable/commit/ee9c0da80 + ALIASES.put("PubkeyAcceptedKeyTypes", //$NON-NLS-1$ + SshConstants.PUBKEY_ACCEPTED_ALGORITHMS); + } + private Map options; private Map> multiOptions; private Map> listOptions; + private static String toKey(String key) { + String k = ALIASES.get(key); + return k != null ? k : key; + } + /** * Retrieves the value of a single-valued key, or the first if the key * has multiple values. Keys are case-insensitive, so @@ -501,15 +527,15 @@ public static class HostEntry implements SshConfigStore.HostConfig { */ @Override public String getValue(String key) { - String result = options != null ? options.get(key) : null; + String k = toKey(key); + String result = options != null ? options.get(k) : null; if (result == null) { // Let's be lenient and return at least the first value from // a list-valued or multi-valued key. - List values = listOptions != null ? listOptions.get(key) + List values = listOptions != null ? listOptions.get(k) : null; if (values == null) { - values = multiOptions != null ? multiOptions.get(key) - : null; + values = multiOptions != null ? multiOptions.get(k) : null; } if (values != null && !values.isEmpty()) { result = values.get(0); @@ -529,10 +555,11 @@ public String getValue(String key) { */ @Override public List getValues(String key) { - List values = listOptions != null ? listOptions.get(key) + String k = toKey(key); + List values = listOptions != null ? listOptions.get(k) : null; if (values == null) { - values = multiOptions != null ? multiOptions.get(key) : null; + values = multiOptions != null ? multiOptions.get(k) : null; } if (values == null || values.isEmpty()) { return new ArrayList<>(); @@ -551,34 +578,35 @@ public List getValues(String key) { * to set or add */ public void setValue(String key, String value) { + String k = toKey(key); if (value == null) { if (multiOptions != null) { - multiOptions.remove(key); + multiOptions.remove(k); } if (listOptions != null) { - listOptions.remove(key); + listOptions.remove(k); } if (options != null) { - options.remove(key); + options.remove(k); } return; } - if (MULTI_KEYS.contains(key)) { + if (MULTI_KEYS.contains(k)) { if (multiOptions == null) { multiOptions = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); } - List values = multiOptions.get(key); + List values = multiOptions.get(k); if (values == null) { values = new ArrayList<>(4); - multiOptions.put(key, values); + multiOptions.put(k, values); } values.add(value); } else { if (options == null) { options = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); } - if (!options.containsKey(key)) { - options.put(key, value); + if (!options.containsKey(k)) { + options.put(k, value); } } } @@ -595,20 +623,21 @@ public void setValue(String key, List values) { if (values.isEmpty()) { return; } + String k = toKey(key); // Check multi-valued keys first; because of the replacement // strategy, they must take precedence over list-valued keys // which always follow the "first occurrence wins" strategy. // // Note that SendEnv is a multi-valued list-valued key. (It's // rather immaterial for JGit, though.) - if (MULTI_KEYS.contains(key)) { + if (MULTI_KEYS.contains(k)) { if (multiOptions == null) { multiOptions = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); } - List items = multiOptions.get(key); + List items = multiOptions.get(k); if (items == null) { items = new ArrayList<>(values); - multiOptions.put(key, items); + multiOptions.put(k, items); } else { items.addAll(values); } @@ -616,8 +645,8 @@ public void setValue(String key, List values) { if (listOptions == null) { listOptions = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); } - if (!listOptions.containsKey(key)) { - listOptions.put(key, values); + if (!listOptions.containsKey(k)) { + listOptions.put(k, values); } } } @@ -630,7 +659,7 @@ public void setValue(String key, List values) { * @return {@code true} if the key is a list-valued key. */ public static boolean isListKey(String key) { - return LIST_KEYS.contains(key.toUpperCase(Locale.ROOT)); + return LIST_KEYS.contains(toKey(key)); } void merge(HostEntry entry) {