From 3f745e40a131fee439119e2c93d750688a4e47e6 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 19 Nov 2018 16:00:30 -0800 Subject: [PATCH] Add a method to get all values of HTTP header defined as list According to RFC 2616 [1] header field names are case insensitive. Header fields defined as a comma separated list can have multiple header fields with the same field name. Add a method to HttpConnection which retrieves all values with a given header field name with the field name compared case insensitive. [1] https://tools.ietf.org/html/rfc2616#section-4.2" Change-Id: I7f601b21cda99e84f43f866c7c7cb4cb0e3cf5c3 Signed-off-by: Matthias Sohn --- .../META-INF/MANIFEST.MF | 1 + .../http/apache/HttpClientConnection.java | 12 +- org.eclipse.jgit.test/META-INF/MANIFEST.MF | 6 + org.eclipse.jgit.test/pom.xml | 6 + .../transport/http/JDKHttpConnectionTest.java | 123 ++++++++++++++++++ org.eclipse.jgit/.settings/.api_filters | 8 ++ .../jgit/transport/http/HttpConnection.java | 29 ++++- .../transport/http/JDKHttpConnection.java | 25 +++- 8 files changed, 205 insertions(+), 5 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/http/JDKHttpConnectionTest.java diff --git a/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF index 1e8842bb7..07778e8c5 100644 --- a/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF @@ -23,6 +23,7 @@ Import-Package: org.apache.http;version="[4.3.0,5.0.0)", org.apache.http.impl.client;version="[4.3.0,5.0.0)", org.apache.http.impl.conn;version="[4.3.0,5.0.0)", org.apache.http.params;version="[4.3.0,5.0.0)", + org.eclipse.jgit.annotations;version="[5.2.0,5.3.0)", org.eclipse.jgit.nls;version="[5.2.0,5.3.0)", org.eclipse.jgit.transport.http;version="[5.2.0,5.3.0)", org.eclipse.jgit.util;version="[5.2.0,5.3.0)" diff --git a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java index 77c5dc0f3..4ac81a54d 100644 --- a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java +++ b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java @@ -58,10 +58,13 @@ import java.net.URL; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.KeyManager; @@ -90,6 +93,7 @@ import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; import org.apache.http.impl.conn.BasicHttpClientConnectionManager; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.transport.http.HttpConnection; import org.eclipse.jgit.transport.http.apache.internal.HttpApacheText; import org.eclipse.jgit.util.TemporaryBuffer; @@ -347,11 +351,17 @@ public class HttpClientConnection implements HttpConnection { // will return only the first field /** {@inheritDoc} */ @Override - public String getHeaderField(String name) { + public String getHeaderField(@NonNull String name) { Header header = resp.getFirstHeader(name); return (header == null) ? null : header.getValue(); } + @Override + public List getHeaderFields(@NonNull String name) { + return Collections.unmodifiableList(Arrays.asList(resp.getHeaders(name)) + .stream().map(Header::getValue).collect(Collectors.toList())); + } + /** {@inheritDoc} */ @Override public int getContentLength() { diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF index 94681178e..863b3bc8f 100644 --- a/org.eclipse.jgit.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.test/META-INF/MANIFEST.MF @@ -10,6 +10,7 @@ Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: JavaSE-1.8 Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)", com.jcraft.jsch;version="[0.1.54,0.2.0)", + net.bytebuddy.dynamic.loading;version="[1.7.0,2.0.0)", org.eclipse.jgit.annotations;version="[5.2.0,5.3.0)", org.eclipse.jgit.api;version="[5.2.0,5.3.0)", org.eclipse.jgit.api.errors;version="[5.2.0,5.3.0)", @@ -63,6 +64,11 @@ Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)", org.junit.rules;version="[4.12,5.0.0)", org.junit.runner;version="[4.12,5.0.0)", org.junit.runners;version="[4.12,5.0.0)", + org.mockito;version="[2.13.0,3.0.0)", + org.mockito.invocation;version="[2.13.0,3.0.0)", + org.mockito.junit;version="[2.13.0,3.0.0)", + org.mockito.stubbing;version="[2.13.0,3.0.0)", + org.objenesis;version="[2.6.0,3.0.0)", org.slf4j;version="[1.7.0,2.0.0)" Require-Bundle: org.hamcrest.core;bundle-version="[1.1.0,2.0.0)", org.hamcrest.library;bundle-version="[1.1.0,2.0.0)" diff --git a/org.eclipse.jgit.test/pom.xml b/org.eclipse.jgit.test/pom.xml index 014f2ceb0..bab7c0309 100644 --- a/org.eclipse.jgit.test/pom.xml +++ b/org.eclipse.jgit.test/pom.xml @@ -89,6 +89,12 @@ [1.1.0,2.0.0) + + org.mockito + mockito-core + 2.13.0 + + org.eclipse.jgit org.eclipse.jgit diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/http/JDKHttpConnectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/http/JDKHttpConnectionTest.java new file mode 100644 index 000000000..10ee82919 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/http/JDKHttpConnectionTest.java @@ -0,0 +1,123 @@ +/* + * Copyright (C) 2018 Matthias Sohn + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.transport.http; + +import static java.util.Arrays.asList; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.net.HttpURLConnection; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; + +public class JDKHttpConnectionTest { + + private Map> headers = new HashMap<>(); + + private HttpURLConnection u; + + private JDKHttpConnection c; + + @Before + public void setup() { + u = mock(HttpURLConnection.class); + c = new JDKHttpConnection(u); + headers.put("ABC", asList("x")); + } + + @Test + public void testSingle() { + when(u.getHeaderFields()).thenReturn(headers); + assertValues("AbC", "x"); + } + + @Test + public void testMultiple1() { + headers.put("abc", asList("a")); + headers.put("aBC", asList("d", "e")); + headers.put("ABc", Collections.emptyList()); + headers.put("AbC", (List) null); + when(u.getHeaderFields()).thenReturn(headers); + assertValues("AbC", "a", "d", "e", "x"); + } + + @Test + public void testMultiple2() { + headers.put("ab", asList("y", "z", "z")); + when(u.getHeaderFields()).thenReturn(headers); + assertValues("ab", "z", "y", "z"); + assertValues("abc", "x"); + assertValues("aBc", "x"); + assertValues("AbCd"); + } + + @Test + public void testCommaSeparatedList() { + headers.put("abc", asList("a,b,c", "d")); + when(u.getHeaderFields()).thenReturn(headers); + assertValues("Abc", "a,b,c", "x", "d"); + } + + private void assertValues(String key, String... values) { + List l = new LinkedList<>(); + List hf = c.getHeaderFields(key); + if (hf != null) { + l.addAll(hf); + } + for (String v : values) { + if (!l.remove(v)) { + fail("value " + v + " not found"); + } + } + assertTrue("found unexpected entries " + l, l.isEmpty()); + } + +} diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 9e8cecbd7..2304fa35b 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -66,4 +66,12 @@ + + + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java index 92965f855..c38b00287 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java @@ -58,6 +58,8 @@ import javax.net.ssl.HostnameVerifier; import javax.net.ssl.KeyManager; import javax.net.ssl.TrustManager; +import org.eclipse.jgit.annotations.NonNull; + /** * The interface of connections used during HTTP communication. This interface * is that subset of the interface exposed by {@link java.net.HttpURLConnection} @@ -139,7 +141,7 @@ public interface HttpConnection { String getResponseMessage() throws IOException; /** - * Get list of header fields + * Get map of header fields * * @see HttpURLConnection#getHeaderFields() * @return a Map of header fields @@ -225,7 +227,13 @@ public interface HttpConnection { InputStream getInputStream() throws IOException; /** - * Get header field + * Get header field. According to + * {@link RFC + * 2616} header field names are case insensitive. Header fields defined + * as a comma separated list can have multiple header fields with the same + * field name. This method only returns one of these header fields. If you + * want the union of all values of all multiple header fields with the same + * field name then use {@link #getHeaderFields(String)} * * @see HttpURLConnection#getHeaderField(String) * @param name @@ -233,7 +241,22 @@ public interface HttpConnection { * @return the value of the named header field, or null if * there is no such field in the header. */ - String getHeaderField(String name); + String getHeaderField(@NonNull String name); + + /** + * Get all values of given header field. According to + * {@link RFC + * 2616} header field names are case insensitive. Header fields defined + * as a comma separated list can have multiple header fields with the same + * field name. This method does not validate if the given header field is + * defined as a comma separated list. + * + * @param name + * the name of a header field. + * @return the list of values of the named header field + * @since 5.2 + */ + List getHeaderFields(@NonNull String name); /** * Get content length diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java index 8241c59d2..734b54929 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java @@ -53,6 +53,7 @@ import java.net.URL; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; +import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -62,6 +63,7 @@ import javax.net.ssl.KeyManager; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManager; +import org.eclipse.jgit.annotations.NonNull; /** * A {@link org.eclipse.jgit.transport.http.HttpConnection} which simply * delegates every call to a {@link java.net.HttpURLConnection}. This is the @@ -72,6 +74,11 @@ import javax.net.ssl.TrustManager; public class JDKHttpConnection implements HttpConnection { HttpURLConnection wrappedUrlConnection; + // used for mock testing + JDKHttpConnection(HttpURLConnection urlConnection) { + this.wrappedUrlConnection = urlConnection; + } + /** * Constructor for JDKHttpConnection. * @@ -170,10 +177,26 @@ public class JDKHttpConnection implements HttpConnection { /** {@inheritDoc} */ @Override - public String getHeaderField(String name) { + public String getHeaderField(@NonNull String name) { return wrappedUrlConnection.getHeaderField(name); } + @Override + public List getHeaderFields(@NonNull String name) { + Map> m = wrappedUrlConnection.getHeaderFields(); + List fields = mapValuesToListIgnoreCase(name, m); + return fields; + } + + private static List mapValuesToListIgnoreCase(String keyName, + Map> m) { + List fields = new LinkedList<>(); + m.entrySet().stream().filter(e -> keyName.equalsIgnoreCase(e.getKey())) + .filter(e -> e.getValue() != null) + .forEach(e -> fields.addAll(e.getValue())); + return fields; + } + /** {@inheritDoc} */ @Override public int getContentLength() {