diff options
author | Bjørn Christian Seime <bjorn.christian@seime.no> | 2019-03-11 11:45:17 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-03-11 11:45:17 +0100 |
commit | 48ce50681ad29a1a17446dbb1f0413615ca35725 (patch) | |
tree | e75670d9e14b94ded5d4a00b0654eee6d8c5a177 | |
parent | d685b1e2fe76a1b0ad064cd34814fa2bf56d2c58 (diff) | |
parent | cc70ba5c9ca4b3873cffe90d572d3a371583190b (diff) |
Merge pull request #8729 from vespa-engine/bjorncs/jdisc-uri-bindings
Use local port when constructing request uri (MERGEOK)
2 files changed, 39 insertions, 467 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java index 617e081bd24..1306d72a618 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java @@ -5,6 +5,7 @@ import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.servlet.ServletRequest; import com.yahoo.jdisc.service.CurrentContainer; +import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.Utf8Appendable; import javax.servlet.http.HttpServletRequest; @@ -37,10 +38,17 @@ class HttpRequestFactory { } } + // Implementation based on org.eclipse.jetty.server.Request.getRequestURL(), but with getLocalPort() as port public static URI getUri(HttpServletRequest servletRequest) { - String query = servletRequest.getQueryString(); try { - return URI.create(servletRequest.getRequestURL() + (query != null ? '?' + query : "")); + StringBuffer builder = new StringBuffer(128); + URIUtil.appendSchemeHostPort(builder, servletRequest.getScheme(), servletRequest.getServerName(), servletRequest.getLocalPort()); + builder.append(servletRequest.getRequestURI()); + String query = servletRequest.getQueryString(); + if (query != null) { + builder.append('?').append(query); + } + return URI.create(builder.toString()); } catch (IllegalArgumentException e) { throw createBadQueryException(e); } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java index 6a2e35b617c..f0203529d4b 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java @@ -12,491 +12,31 @@ import com.yahoo.jdisc.service.CurrentContainer; import org.eclipse.jetty.server.HttpConnection; import org.testng.annotations.Test; -import javax.servlet.AsyncContext; -import javax.servlet.DispatcherType; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; -import javax.servlet.http.HttpUpgradeHandler; -import javax.servlet.http.Part; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.net.URI; -import java.security.Principal; -import java.util.Collection; -import java.util.Enumeration; -import java.util.Locale; -import java.util.Map; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.fail; /** - * * @author Steinar Knutsen + * @author bjorncs */ public class HttpRequestFactoryTest { - private static final class MockRequest implements HttpServletRequest { - String queryString = null; - StringBuffer requestUrl; - final String method = "GET"; - final String protocol = "HTTP/1.1"; - final String remoteAddr = "127.0.0.1"; - final int remotePort = 0; - - public MockRequest(String sortOfUri) { - int mark = sortOfUri.indexOf('?'); - if (mark > 0) { - queryString = sortOfUri.substring(mark + 1); - requestUrl = new StringBuffer(sortOfUri.substring(0, mark)); - } else { - queryString = null; - requestUrl = new StringBuffer(sortOfUri); - } - } - - @Override - public Object getAttribute(String name) { - switch (name) { - case "org.eclipse.jetty.server.HttpConnection": - HttpConnection connection = mock(HttpConnection.class); - when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); - return connection; - default: - return null; - } - } - - @Override - public Enumeration<String> getAttributeNames() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getCharacterEncoding() { - // TODO Auto-generated method stub - return null; - } - - @Override - public void setCharacterEncoding(String env) - throws UnsupportedEncodingException { - // TODO Auto-generated method stub - - } - - @Override - public int getContentLength() { - // TODO Auto-generated method stub - return 0; - } - - @Override - public long getContentLengthLong() { - // TODO Auto-generated method stub - return 0; - } - - @Override - public String getContentType() { - // TODO Auto-generated method stub - return null; - } - - @Override - public ServletInputStream getInputStream() throws IOException { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getParameter(String name) { - // TODO Auto-generated method stub - return null; - } - - @Override - public Enumeration<String> getParameterNames() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String[] getParameterValues(String name) { - // TODO Auto-generated method stub - return null; - } - - @Override - public Map<String, String[]> getParameterMap() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getProtocol() { - return protocol; - } - - @Override - public String getScheme() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getServerName() { - // TODO Auto-generated method stub - return null; - } - - @Override - public int getServerPort() { - // TODO Auto-generated method stub - return 0; - } - - @Override - public BufferedReader getReader() throws IOException { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getRemoteAddr() { - return remoteAddr; - } - - @Override - public String getRemoteHost() { - // TODO Auto-generated method stub - return null; - } - - @Override - public void setAttribute(String name, Object o) { - // TODO Auto-generated method stub - - } - - @Override - public void removeAttribute(String name) { - // TODO Auto-generated method stub - - } - - @Override - public Locale getLocale() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Enumeration<Locale> getLocales() { - // TODO Auto-generated method stub - return null; - } - - @Override - public boolean isSecure() { - // TODO Auto-generated method stub - return false; - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) { - // TODO Auto-generated method stub - return null; - } - - @Override - @Deprecated - public String getRealPath(String path) { - // TODO Auto-generated method stub - return null; - } - - @Override - public int getRemotePort() { - return remotePort; - } - - @Override - public String getLocalName() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getLocalAddr() { - // TODO Auto-generated method stub - return null; - } - - @Override - public int getLocalPort() { - // TODO Auto-generated method stub - return 0; - } - - @Override - public ServletContext getServletContext() { - // TODO Auto-generated method stub - return null; - } - - @Override - public AsyncContext startAsync() throws IllegalStateException { - // TODO Auto-generated method stub - return null; - } - - @Override - public AsyncContext startAsync(ServletRequest servletRequest, - ServletResponse servletResponse) throws IllegalStateException { - // TODO Auto-generated method stub - return null; - } - - @Override - public boolean isAsyncStarted() { - // TODO Auto-generated method stub - return false; - } - - @Override - public boolean isAsyncSupported() { - // TODO Auto-generated method stub - return false; - } - - @Override - public AsyncContext getAsyncContext() { - // TODO Auto-generated method stub - return null; - } - - @Override - public DispatcherType getDispatcherType() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getAuthType() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Cookie[] getCookies() { - // TODO Auto-generated method stub - return null; - } - - @Override - public long getDateHeader(String name) { - // TODO Auto-generated method stub - return 0; - } - - @Override - public String getHeader(String name) { - // TODO Auto-generated method stub - return null; - } - - @Override - public Enumeration<String> getHeaders(String name) { - // TODO Auto-generated method stub - return null; - } - - @Override - public Enumeration<String> getHeaderNames() { - // TODO Auto-generated method stub - return null; - } - - @Override - public int getIntHeader(String name) { - // TODO Auto-generated method stub - return 0; - } - - @Override - public String getMethod() { - return method; - } - - @Override - public String getPathInfo() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getPathTranslated() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getContextPath() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getQueryString() { - return queryString; - } - - @Override - public String getRemoteUser() { - // TODO Auto-generated method stub - return null; - } - - @Override - public boolean isUserInRole(String role) { - // TODO Auto-generated method stub - return false; - } - - @Override - public Principal getUserPrincipal() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getRequestedSessionId() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String getRequestURI() { - // TODO Auto-generated method stub - return null; - } - - @Override - public StringBuffer getRequestURL() { - return requestUrl; - } - - @Override - public String getServletPath() { - // TODO Auto-generated method stub - return null; - } - - @Override - public HttpSession getSession(boolean create) { - // TODO Auto-generated method stub - return null; - } - - @Override - public HttpSession getSession() { - // TODO Auto-generated method stub - return null; - } - - @Override - public String changeSessionId() { - // TODO Auto-generated method stub - return null; - } - - @Override - public boolean isRequestedSessionIdValid() { - // TODO Auto-generated method stub - return false; - } - - @Override - public boolean isRequestedSessionIdFromCookie() { - // TODO Auto-generated method stub - return false; - } - - @Override - public boolean isRequestedSessionIdFromURL() { - // TODO Auto-generated method stub - return false; - } - - @Override - @Deprecated - public boolean isRequestedSessionIdFromUrl() { - // TODO Auto-generated method stub - return false; - } - - @Override - public boolean authenticate(HttpServletResponse response) - throws IOException, ServletException { - // TODO Auto-generated method stub - return false; - } - - @Override - public void login(String username, String password) - throws ServletException { - // TODO Auto-generated method stub - - } - - @Override - public void logout() throws ServletException { - // TODO Auto-generated method stub - - } - - @Override - public Collection<Part> getParts() throws IOException, ServletException { - // TODO Auto-generated method stub - return null; - } - - @Override - public Part getPart(String name) throws IOException, ServletException { - // TODO Auto-generated method stub - return null; - } - - @Override - public <T extends HttpUpgradeHandler> T upgrade(Class<T> handlerClass) - throws IOException, ServletException { - // TODO Auto-generated method stub - return null; - } - - } + private static final int LOCAL_PORT = 8080; @Test public final void testIllegalQuery() { try { HttpRequestFactory.newJDiscRequest( new MockContainer(), - new MockRequest("http://example.com/search?query=\"contains_quotes\"")); + createMockRequest("http", "example.com", "/search", "query=\"contains_quotes\"")); fail("Above statement should throw"); } catch (RequestException e) { assertThat(e.getResponseStatus(), is(Response.Status.BAD_REQUEST)); @@ -508,7 +48,7 @@ public class HttpRequestFactoryTest { try { HttpRequestFactory.newJDiscRequest( new MockContainer(), - new MockRequest("http://example.com/search?query=%c0%ae")); + createMockRequest("http", "example.com", "/search", "query=%c0%ae")); fail("Above statement should throw"); } catch (RequestException e) { assertThat(e.getResponseStatus(), is(Response.Status.BAD_REQUEST)); @@ -516,6 +56,30 @@ public class HttpRequestFactoryTest { } } + @Test + public void request_uri_uses_local_port() { + HttpRequest request = HttpRequestFactory.newJDiscRequest( + new MockContainer(), + createMockRequest("http", "example.com", "/search", "query=value")); + assertEquals(LOCAL_PORT, request.getUri().getPort()); + } + + private static HttpServletRequest createMockRequest(String scheme, String serverName, String path, String queryString) { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpConnection connection = mock(HttpConnection.class); + when(connection.getCreatedTimeStamp()).thenReturn(System.currentTimeMillis()); + when(request.getAttribute("org.eclipse.jetty.server.HttpConnection")).thenReturn(connection); + when(request.getProtocol()).thenReturn("HTTP/1.1"); + when(request.getScheme()).thenReturn(scheme); + when(request.getServerName()).thenReturn(serverName); + when(request.getRemoteAddr()).thenReturn("127.0.0.1"); + when(request.getRemotePort()).thenReturn(1234); + when(request.getLocalPort()).thenReturn(LOCAL_PORT); + when(request.getMethod()).thenReturn("GET"); + when(request.getQueryString()).thenReturn(queryString); + when(request.getRequestURI()).thenReturn(path); + return request; + } private static final class MockContainer implements CurrentContainer { |