From 2171f868d95f110024e90de635627f14a3fb4700 Mon Sep 17 00:00:00 2001 From: James Wynn Date: Mon, 31 Aug 2020 15:32:13 -0500 Subject: [PATCH] Support "http.userAgent" and "http.extraHeader" from the git config Validate the extra headers and log but otherwise ignore invalid headers. An empty http.extraHeader starts the list afresh. The http.userAgent is restricted to printable 7-bit ASCII, other characters are replaced by '.'. Moves a support method from the ssh.apache bundle to HttpSupport in the main JGit bundle. Bug:541500 Change-Id: Id2d8df12914e2cdbd936ff00dc824d8f871bd580 Signed-off-by: James Wynn Signed-off-by: Thomas Wolf --- .../transport/sshd/proxy/HttpParser.java | 51 ++---------- .../jgit/transport/HttpConfigTest.java | 65 +++++++++++++++ .../jgit/transport/TransportHttpTest.java | 48 +++++++++++ org.eclipse.jgit/.settings/.api_filters | 14 ++++ .../eclipse/jgit/internal/JGitText.properties | 3 + .../org/eclipse/jgit/internal/JGitText.java | 3 + .../eclipse/jgit/transport/HttpConfig.java | 82 +++++++++++++++++++ .../eclipse/jgit/transport/TransportHttp.java | 44 +++++++++- .../org/eclipse/jgit/transport/UserAgent.java | 2 +- .../org/eclipse/jgit/util/HttpSupport.java | 63 ++++++++++++++ 10 files changed, 327 insertions(+), 48 deletions(-) diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/proxy/HttpParser.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/proxy/HttpParser.java index d5b80374c..0500a6342 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/proxy/HttpParser.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/proxy/HttpParser.java @@ -13,6 +13,8 @@ import java.util.Iterator; import java.util.List; +import org.eclipse.jgit.util.HttpSupport; + /** * A basic parser for HTTP response headers. Handles status lines and * authentication headers (WWW-Authenticate, Proxy-Authenticate). @@ -135,7 +137,7 @@ private static void parseChallenges( int length = header.length(); for (int i = 0; i < length;) { int start = skipWhiteSpace(header, i); - int end = scanToken(header, start); + int end = HttpSupport.scanToken(header, start); if (end <= start) { break; } @@ -156,7 +158,7 @@ private static int parseChallenge(AuthenticationChallenge challenge, // optional legacy whitespace around the equals sign), where the // value can be either a token or a quoted string. start = skipWhiteSpace(header, start); - int end = scanToken(header, start); + int end = HttpSupport.scanToken(header, start); if (end == start) { // Nothing found. Either at end or on a comma. if (start < header.length() && header.charAt(start) == ',') { @@ -222,7 +224,7 @@ private static int parseChallenge(AuthenticationChallenge challenge, challenge.addArgument(header.substring(start, end), value); start = nextEnd[0]; } else { - int nextEnd = scanToken(header, nextStart); + int nextEnd = HttpSupport.scanToken(header, nextStart); challenge.addArgument(header.substring(start, end), header.substring(nextStart, nextEnd)); start = nextEnd; @@ -244,49 +246,6 @@ private static int skipWhiteSpace(String header, int i) { return i; } - private static int scanToken(String header, int from) { - int length = header.length(); - int i = from; - while (i < length) { - char c = header.charAt(i); - switch (c) { - case '!': - case '#': - case '$': - case '%': - case '&': - case '\'': - case '*': - case '+': - case '-': - case '.': - case '^': - case '_': - case '`': - case '|': - case '0': - case '1': - case '2': - case '3': - case '4': - case '5': - case '6': - case '7': - case '8': - case '9': - i++; - break; - default: - if (c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z') { - i++; - break; - } - return i; - } - } - return i; - } - private static String scanQuotedString(String header, int from, int[] to) { StringBuilder result = new StringBuilder(); int length = header.length(); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/HttpConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/HttpConfigTest.java index fcbec52a5..a5f98ee23 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/HttpConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/HttpConfigTest.java @@ -25,6 +25,7 @@ public class HttpConfigTest { private static final String DEFAULT = "[http]\n" + "\tpostBuffer = 1\n" + "\tsslVerify= true\n" + "\tfollowRedirects = true\n" + + "\textraHeader = x: y\n" + "\tuserAgent = Test/0.1\n" + "\tmaxRedirects = 5\n\n"; private Config config; @@ -174,4 +175,68 @@ public void testMatchLonger() throws Exception { new URIish("http://user@example.com/path")); assertEquals(1024, http.getPostBuffer()); } + + @Test + public void testExtraHeaders() throws Exception { + config.fromText(DEFAULT + "[http \"http://example.com\"]\n" + + "\textraHeader=foo: bar\n"); + HttpConfig http = new HttpConfig(config, + new URIish("http://example.com/")); + assertEquals(1, http.getExtraHeaders().size()); + assertEquals("foo: bar", http.getExtraHeaders().get(0)); + } + + @Test + public void testExtraHeadersMultiple() throws Exception { + config.fromText(DEFAULT + "[http \"http://example.com\"]\n" + + "\textraHeader=foo: bar\n" // + + "\textraHeader=bar: foo\n"); + HttpConfig http = new HttpConfig(config, + new URIish("http://example.com/")); + assertEquals(2, http.getExtraHeaders().size()); + assertEquals("foo: bar", http.getExtraHeaders().get(0)); + assertEquals("bar: foo", http.getExtraHeaders().get(1)); + } + + @Test + public void testExtraHeadersReset() throws Exception { + config.fromText(DEFAULT + "[http \"http://example.com\"]\n" + + "\textraHeader=foo: bar\n" // + + "\textraHeader=bar: foo\n" // + + "\textraHeader=\n"); + HttpConfig http = new HttpConfig(config, + new URIish("http://example.com/")); + assertTrue(http.getExtraHeaders().isEmpty()); + } + + @Test + public void testExtraHeadersResetAndMore() throws Exception { + config.fromText(DEFAULT + "[http \"http://example.com\"]\n" + + "\textraHeader=foo: bar\n" // + + "\textraHeader=bar: foo\n" // + + "\textraHeader=\n" // + + "\textraHeader=baz: something\n"); + HttpConfig http = new HttpConfig(config, + new URIish("http://example.com/")); + assertEquals(1, http.getExtraHeaders().size()); + assertEquals("baz: something", http.getExtraHeaders().get(0)); + } + + @Test + public void testUserAgent() throws Exception { + config.fromText(DEFAULT + "[http \"http://example.com\"]\n" + + "\tuserAgent=DummyAgent/4.0\n"); + HttpConfig http = new HttpConfig(config, + new URIish("http://example.com/")); + assertEquals("DummyAgent/4.0", http.getUserAgent()); + } + + @Test + public void testUserAgentNonAscii() throws Exception { + config.fromText(DEFAULT + "[http \"http://example.com\"]\n" + + "\tuserAgent= d ümmy Agent -5.10\n"); + HttpConfig http = new HttpConfig(config, + new URIish("http://example.com/")); + assertEquals("d.mmy.Agent.-5.10", http.getUserAgent()); + } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportHttpTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportHttpTest.java index b84b6b2e0..029b45e1e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportHttpTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportHttpTest.java @@ -18,7 +18,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.Date; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; +import java.util.Map; import java.util.Set; import org.eclipse.jgit.internal.transport.http.NetscapeCookieFile; @@ -155,4 +157,50 @@ public void testProcessResponseCookiesNotPersistingWithSaveCookiesFalse() cookieFile.exists()); } } + + private void assertHeaders(String expected, String... headersToAdd) { + HttpConnection fake = Mockito.mock(HttpConnection.class); + Map headers = new LinkedHashMap<>(); + Mockito.doAnswer(invocation -> { + Object[] args = invocation.getArguments(); + headers.put(args[0].toString(), args[1].toString()); + return null; + }).when(fake).setRequestProperty(ArgumentMatchers.anyString(), + ArgumentMatchers.anyString()); + TransportHttp.addHeaders(fake, Arrays.asList(headersToAdd)); + Assert.assertEquals(expected, headers.toString()); + } + + @Test + public void testAddHeaders() { + assertHeaders("{a=b, c=d}", "a: b", "c :d"); + } + + @Test + public void testAddHeaderEmptyValue() { + assertHeaders("{a-x=b, c=, d=e}", "a-x: b", "c:", "d:e"); + } + + @Test + public void testSkipHeaderWithEmptyKey() { + assertHeaders("{a=b, c=d}", "a: b", " : x", "c :d"); + assertHeaders("{a=b, c=d}", "a: b", ": x", "c :d"); + } + + @Test + public void testSkipHeaderWithoutKey() { + assertHeaders("{a=b, c=d}", "a: b", "x", "c :d"); + } + + @Test + public void testSkipHeaderWithInvalidKey() { + assertHeaders("{a=b, c=d}", "a: b", "q/p: x", "c :d"); + assertHeaders("{a=b, c=d}", "a: b", "ä: x", "c :d"); + } + + @Test + public void testSkipHeaderWithNonAsciiValue() { + assertHeaders("{a=b, c=d}", "a: b", "q/p: x", "c :d"); + assertHeaders("{a=b, c=d}", "a: b", "x: ä", "c :d"); + } } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 05fc371c8..538c6f7bf 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -22,4 +22,18 @@ + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index beafff381..c9e48a5c0 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -347,6 +347,9 @@ invalidFilter=Invalid filter: {0} invalidGitdirRef = Invalid .git reference in file ''{0}'' invalidGitModules=Invalid .gitmodules file invalidGitType=invalid git type: {0} +invalidHeaderFormat=Invalid header from git config http.extraHeader ignored: no colon or empty key in header ''{0}'' +invalidHeaderKey=Invalid header from git config http.extraHeader ignored: key contains illegal characters; see RFC 7230: ''{0}'' +invalidHeaderValue=Invalid header from git config http.extraHeader ignored: value should be 7bit-ASCII characters only: ''{0}'' invalidHexString=Invalid hex string: {0} invalidHomeDirectory=Invalid home directory: {0} invalidHooksPath=Invalid git config core.hooksPath = {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 3f2565fdd..b761210e6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -375,6 +375,9 @@ public static JGitText get() { /***/ public String invalidGitdirRef; /***/ public String invalidGitModules; /***/ public String invalidGitType; + /***/ public String invalidHeaderFormat; + /***/ public String invalidHeaderKey; + /***/ public String invalidHeaderValue; /***/ public String invalidHexString; /***/ public String invalidHomeDirectory; /***/ public String invalidHooksPath; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java index 79cba80e1..58fc25025 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java @@ -14,9 +14,13 @@ import java.io.IOException; import java.net.URISyntaxException; import java.text.MessageFormat; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.function.Supplier; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Config; @@ -55,6 +59,20 @@ public class HttpConfig { /** git config key for the "sslVerify" setting. */ public static final String SSL_VERIFY_KEY = "sslVerify"; //$NON-NLS-1$ + /** + * git config key for the "userAgent" setting. + * + * @since 5.10 + */ + public static final String USER_AGENT = "userAgent"; //$NON-NLS-1$ + + /** + * git config key for the "extraHeader" setting. + * + * @since 5.10 + */ + public static final String EXTRA_HEADER = "extraHeader"; //$NON-NLS-1$ + /** * git config key for the "cookieFile" setting. * @@ -143,6 +161,10 @@ public boolean matchConfigValue(String s) { private int maxRedirects; + private String userAgent; + + private List extraHeaders; + private String cookieFile; private boolean saveCookies; @@ -185,6 +207,27 @@ public int getMaxRedirects() { return maxRedirects; } + /** + * Get the "http.userAgent" setting + * + * @return the value of the "http.userAgent" setting + * @since 5.10 + */ + public String getUserAgent() { + return userAgent; + } + + /** + * Get the "http.extraHeader" setting + * + * @return the value of the "http.extraHeader" setting + * @since 5.10 + */ + @NonNull + public List getExtraHeaders() { + return extraHeaders == null ? Collections.emptyList() : extraHeaders; + } + /** * Get the "http.cookieFile" setting * @@ -265,11 +308,25 @@ private void init(Config config, URIish uri) { if (redirectLimit < 0) { redirectLimit = MAX_REDIRECTS; } + String agent = config.getString(HTTP, null, USER_AGENT); + if (agent != null) { + agent = UserAgent.clean(agent); + } + userAgent = agent; + String[] headers = config.getStringList(HTTP, null, EXTRA_HEADER); + // https://git-scm.com/docs/git-config#Documentation/git-config.txt-httpextraHeader + // "an empty value will reset the extra headers to the empty list." + int start = findLastEmpty(headers) + 1; + if (start > 0) { + headers = Arrays.copyOfRange(headers, start, headers.length); + } + extraHeaders = Arrays.asList(headers); cookieFile = config.getString(HTTP, null, COOKIE_FILE_KEY); saveCookies = config.getBoolean(HTTP, SAVE_COOKIES_KEY, false); cookieFileCacheLimit = config.getInt(HTTP, COOKIE_FILE_CACHE_LIMIT_KEY, DEFAULT_COOKIE_FILE_CACHE_LIMIT); String match = findMatch(config.getSubsections(HTTP), uri); + if (match != null) { // Override with more specific items postBufferSize = config.getInt(HTTP, match, POST_BUFFER_KEY, @@ -283,6 +340,22 @@ private void init(Config config, URIish uri) { if (newMaxRedirects >= 0) { redirectLimit = newMaxRedirects; } + String uriSpecificUserAgent = config.getString(HTTP, match, + USER_AGENT); + if (uriSpecificUserAgent != null) { + userAgent = UserAgent.clean(uriSpecificUserAgent); + } + String[] uriSpecificExtraHeaders = config.getStringList(HTTP, match, + EXTRA_HEADER); + if (uriSpecificExtraHeaders.length > 0) { + start = findLastEmpty(uriSpecificExtraHeaders) + 1; + if (start > 0) { + uriSpecificExtraHeaders = Arrays.copyOfRange( + uriSpecificExtraHeaders, start, + uriSpecificExtraHeaders.length); + } + extraHeaders = Arrays.asList(uriSpecificExtraHeaders); + } String urlSpecificCookieFile = config.getString(HTTP, match, COOKIE_FILE_KEY); if (urlSpecificCookieFile != null) { @@ -297,6 +370,15 @@ private void init(Config config, URIish uri) { maxRedirects = redirectLimit; } + private int findLastEmpty(String[] values) { + for (int i = values.length - 1; i >= 0; i--) { + if (values[i] == null) { + return i; + } + } + return -1; + } + /** * Determines the best match from a set of subsection names (representing * prefix URLs) for the given {@link URIish}. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java index 16169f028..6768387e6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java @@ -49,6 +49,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; @@ -900,7 +901,9 @@ protected HttpConnection httpOpen(String method, URL u, conn.setRequestProperty(HDR_ACCEPT_ENCODING, ENCODING_GZIP); } conn.setRequestProperty(HDR_PRAGMA, "no-cache"); //$NON-NLS-1$ - if (UserAgent.get() != null) { + if (http.getUserAgent() != null) { + conn.setRequestProperty(HDR_USER_AGENT, http.getUserAgent()); + } else if (UserAgent.get() != null) { conn.setRequestProperty(HDR_USER_AGENT, UserAgent.get()); } int timeOut = getTimeout(); @@ -909,6 +912,7 @@ protected HttpConnection httpOpen(String method, URL u, conn.setConnectTimeout(effTimeOut); conn.setReadTimeout(effTimeOut); } + addHeaders(conn, http.getExtraHeaders()); // set cookie header if necessary if (!relevantCookies.isEmpty()) { setCookieHeader(conn); @@ -923,6 +927,44 @@ protected HttpConnection httpOpen(String method, URL u, return conn; } + /** + * Adds a list of header strings to the connection. Headers are expected to + * separate keys from values, i.e. "Key: Value". Headers without colon or + * key are ignored (and logged), as are headers with keys that are not RFC + * 7230 tokens or with non-ASCII values. + * + * @param conn + * The target HttpConnection + * @param headersToAdd + * A list of header strings + */ + static void addHeaders(HttpConnection conn, List headersToAdd) { + for (String header : headersToAdd) { + // Empty values are allowed according to + // https://tools.ietf.org/html/rfc7230 + int colon = header.indexOf(':'); + String key = null; + if (colon > 0) { + key = header.substring(0, colon).trim(); + } + if (key == null || key.isEmpty()) { + LOG.warn(MessageFormat.format( + JGitText.get().invalidHeaderFormat, header)); + } else if (HttpSupport.scanToken(key, 0) != key.length()) { + LOG.warn(MessageFormat.format(JGitText.get().invalidHeaderKey, + header)); + } else { + String value = header.substring(colon + 1).trim(); + if (!StandardCharsets.US_ASCII.newEncoder().canEncode(value)) { + LOG.warn(MessageFormat + .format(JGitText.get().invalidHeaderValue, header)); + } else { + conn.setRequestProperty(key, value); + } + } + } + } + private void setCookieHeader(HttpConnection conn) { StringBuilder cookieHeaderValue = new StringBuilder(); for (HttpCookie cookie : relevantCookies) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UserAgent.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UserAgent.java index 5b63635e5..604eb3a66 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UserAgent.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UserAgent.java @@ -46,7 +46,7 @@ private static String computeVersion() { return "unknown"; //$NON-NLS-1$ } - private static String clean(String s) { + static String clean(String s) { s = s.trim(); StringBuilder b = new StringBuilder(s.length()); for (int i = 0; i < s.length(); i++) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java index 8ff649bcc..03672f8ba 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java @@ -424,6 +424,69 @@ private static String getProperty(String property) { } } + /** + * Scan a RFC 7230 token as it appears in HTTP headers. + * + * @param header + * to scan in + * @param from + * index in {@code header} to start scanning at + * @return the index after the token, that is, on the first non-token + * character or {@code header.length} + * @throws IndexOutOfBoundsException + * if {@code from < 0} or {@code from > header.length()} + * + * @see RFC 7230, + * Appendix B: Collected Grammar; "token" production + * @since 5.10 + */ + public static int scanToken(String header, int from) { + int length = header.length(); + int i = from; + if (i < 0 || i > length) { + throw new IndexOutOfBoundsException(); + } + while (i < length) { + char c = header.charAt(i); + switch (c) { + case '!': + case '#': + case '$': + case '%': + case '&': + case '\'': + case '*': + case '+': + case '-': + case '.': + case '^': + case '_': + case '`': + case '|': + case '~': + case '0': + case '1': + case '2': + case '3': + case '4': + case '5': + case '6': + case '7': + case '8': + case '9': + i++; + break; + default: + if (c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z') { + i++; + break; + } + return i; + } + } + return i; + } + private HttpSupport() { // Utility class only. }