From 6163f8ce8f699cd16ee1f328f41fc8788b0f7cf2 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 1 Mar 2016 10:00:08 +1000 Subject: [PATCH 1/3] ZNTA-937 - handle server redirect gracefully --- .../rest/client/InvalidContentTypeFilter.java | 74 +++++++++++++++++++ .../zanata/rest/client/RedirectFilter.java | 61 +++++++++++++++ .../zanata/rest/client/RestClientFactory.java | 2 + .../client/InvalidContentTypeFilterTest.java | 37 ++++++++++ 4 files changed, 174 insertions(+) create mode 100644 zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java create mode 100644 zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java create mode 100644 zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java new file mode 100644 index 00000000..1110d78f --- /dev/null +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java @@ -0,0 +1,74 @@ +/* + * Copyright 2015, Red Hat, Inc. and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.zanata.rest.client; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.ws.rs.core.MediaType; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import com.sun.jersey.api.client.ClientHandler; +import com.sun.jersey.api.client.ClientHandlerException; +import com.sun.jersey.api.client.ClientRequest; +import com.sun.jersey.api.client.ClientResponse; +import com.sun.jersey.api.client.filter.ClientFilter; + +/** + * @author Patrick Huang pahuang@redhat.com + */ +public class InvalidContentTypeFilter extends ClientFilter { + private static final Logger log = + LoggerFactory.getLogger(InvalidContentTypeFilter.class); + private static final String ERROR_MSG = "Receiving HTML from the server. " + + "Most likely hitting a redirect or server returning error page. " + + "Please check the server URL is correct (in zanata.ini and in zanata.xml) and make sure you use the non-redirected address."; + + + @Override + public ClientResponse handle(ClientRequest clientRequest) + throws ClientHandlerException { + ClientHandler ch = getNext(); + ClientResponse resp = ch.handle(clientRequest); + + if (MediaType.TEXT_HTML_TYPE.isCompatible(resp.getType())) { + log.error(ERROR_MSG); + String title = findPageTitle(resp); + String snippet = String.format("Wrong content type received: [%s]. Content page title: [%s]", + resp.getType(), title); + + log.error(snippet); + throw new IllegalStateException(snippet); + } else { + return resp; + } + } + + private String findPageTitle(ClientResponse resp) { + String body = resp.getEntity(String.class).replaceAll("\\n", " "); + Pattern pattern = Pattern.compile(".*(.*).*"); + Matcher matcher = pattern.matcher(body); + if (matcher.matches()) { + return matcher.group(1); + } + return ""; + } +} diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java new file mode 100644 index 00000000..f0445577 --- /dev/null +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java @@ -0,0 +1,61 @@ +/* + * Copyright 2015, Red Hat, Inc. and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.zanata.rest.client; + +import java.net.URI; +import javax.ws.rs.core.Response; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import com.sun.jersey.api.client.ClientHandler; +import com.sun.jersey.api.client.ClientHandlerException; +import com.sun.jersey.api.client.ClientRequest; +import com.sun.jersey.api.client.ClientResponse; +import com.sun.jersey.api.client.filter.ClientFilter; + +/** + * @author Patrick Huang pahuang@redhat.com + */ +public class RedirectFilter extends ClientFilter { + private static final Logger log = + LoggerFactory.getLogger(RedirectFilter.class); + + @Override + public ClientResponse handle(ClientRequest clientRequest) + throws ClientHandlerException { + ClientHandler ch = getNext(); + ClientResponse resp = ch.handle(clientRequest); + + if (resp.getClientResponseStatus().getFamily() != + Response.Status.Family.REDIRECTION) { + return resp; + } else { + // try location + log.debug("Server returns redirection status: {}. Try to follow it", + resp.getClientResponseStatus()); + URI redirectTarget = resp.getLocation(); + if (redirectTarget != null) { + clientRequest.setURI(redirectTarget); + } + return ch.handle(clientRequest); + } + } +} diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/RestClientFactory.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/RestClientFactory.java index faac5150..b1f543ee 100644 --- a/zanata-rest-client/src/main/java/org/zanata/rest/client/RestClientFactory.java +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/RestClientFactory.java @@ -81,10 +81,12 @@ public RestClientFactory(URI base, String username, String apiKey, clientConfig.getClasses().add(JacksonJsonProvider.class); client = Client.create(clientConfig); + client.addFilter(new RedirectFilter()); client.addFilter( new ApiKeyHeaderFilter(username, apiKey, clientVersion)); client.addFilter(new AcceptTypeFilter()); client.addFilter(new TraceDebugFilter(logHttp)); + client.addFilter(new InvalidContentTypeFilter()); } private static void sslConfiguration(boolean sslCertDisabled, diff --git a/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java b/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java new file mode 100644 index 00000000..8b28d51e --- /dev/null +++ b/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java @@ -0,0 +1,37 @@ +package org.zanata.rest.client; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.hamcrest.MatcherAssert; +import org.junit.Test; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.*; + +public class InvalidContentTypeFilterTest { + + private String sampleText = + "\n" + + "\n" + + " \n" + + " \n" + + " Zanata: Home\n" + + " \n" + + " \n" + + " \n" + + " "; + + private String text = " Zanata: Home \n"; + + @Test + public void testPatternMatch() { + Pattern pattern = Pattern.compile(".*(.*).*", Pattern.CASE_INSENSITIVE); + + Matcher matcher = pattern.matcher(sampleText.replaceAll("\\n", " ")); + + MatcherAssert.assertThat(matcher.matches(), equalTo(true)); + MatcherAssert.assertThat(matcher.group(1), equalTo("Zanata: Home")); + } + +} From 389276a3af3b4a00c36c48838c3a8b6ce57783e7 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 1 Mar 2016 13:21:03 +1000 Subject: [PATCH 2/3] ZNTA-937 - reject all invalid content type and only follow redirect for get,head --- .../org/zanata/client/HTTPMockContainer.java | 3 +- .../rest/client/InvalidContentTypeFilter.java | 40 ++++++++++++++++--- .../zanata/rest/client/RedirectFilter.java | 24 +++++++---- .../client/InvalidContentTypeFilterTest.java | 38 +++++++++++++++++- 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/zanata-cli/src/test/java/org/zanata/client/HTTPMockContainer.java b/zanata-cli/src/test/java/org/zanata/client/HTTPMockContainer.java index a30ecad7..0c680447 100644 --- a/zanata-cli/src/test/java/org/zanata/client/HTTPMockContainer.java +++ b/zanata-cli/src/test/java/org/zanata/client/HTTPMockContainer.java @@ -38,9 +38,8 @@ public void handle(Request request, Response response) { PrintStream body = response.getPrintStream(); long time = System.currentTimeMillis(); - response.setValue("Content-Type", "text/plain"); response.setStatus(status); - response.setContentType("text/xml;charset=utf-8"); + response.setContentType("text/plain"); response.setDate("Date", time); response.setDate("Last-Modified", time); diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java index 1110d78f..2545a002 100644 --- a/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2015, Red Hat, Inc. and individual contributors + * Copyright 2016, Red Hat, Inc. and individual contributors * as indicated by the @author tags. See the copyright.txt file in the * distribution for a full listing of individual contributors. * @@ -20,12 +20,17 @@ */ package org.zanata.rest.client; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.ws.rs.core.MediaType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.sun.jersey.api.client.ClientHandler; import com.sun.jersey.api.client.ClientHandlerException; import com.sun.jersey.api.client.ClientRequest; @@ -38,21 +43,27 @@ public class InvalidContentTypeFilter extends ClientFilter { private static final Logger log = LoggerFactory.getLogger(InvalidContentTypeFilter.class); - private static final String ERROR_MSG = "Receiving HTML from the server. " + - "Most likely hitting a redirect or server returning error page. " + - "Please check the server URL is correct (in zanata.ini and in zanata.xml) and make sure you use the non-redirected address."; + private static final String ERROR_MSG = + "Received invalid content type from the server. " + + "Most likely you're hitting an error page or being redirected. " + + "Please check the server URL is correct (in zanata.ini and in zanata.xml) and make sure you use the correct address."; + // we assume only xml or json are the valid types (wildcard type is also considered compatible) + private static final Pattern VALID_TYPES_REGEX = + Pattern.compile("application/.*\\+?(\\*|xml|json)"); + @Override public ClientResponse handle(ClientRequest clientRequest) throws ClientHandlerException { ClientHandler ch = getNext(); ClientResponse resp = ch.handle(clientRequest); - if (MediaType.TEXT_HTML_TYPE.isCompatible(resp.getType())) { + if (!isContentTypeCompatible(resp.getType())) { log.error(ERROR_MSG); String title = findPageTitle(resp); - String snippet = String.format("Wrong content type received: [%s]. Content page title: [%s]", + String snippet = String.format( + "Wrong content type received: [%s]. Content page title: [%s]", resp.getType(), title); log.error(snippet); @@ -62,6 +73,23 @@ public ClientResponse handle(ClientRequest clientRequest) } } + @VisibleForTesting + protected static boolean isContentTypeCompatible( + final MediaType responseContentType) { + if (responseContentType == null || + responseContentType.isWildcardType() || + responseContentType.isWildcardSubtype()) { + return true; + } + // a few end points will return text/plain + if (MediaType.TEXT_PLAIN_TYPE.isCompatible(responseContentType)) { + return true; + } + Matcher matcher = + VALID_TYPES_REGEX.matcher(responseContentType.toString()); + return matcher.matches(); + } + private String findPageTitle(ClientResponse resp) { String body = resp.getEntity(String.class).replaceAll("\\n", " "); Pattern pattern = Pattern.compile(".*(.*).*"); diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java index f0445577..8bd5c8e1 100644 --- a/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/RedirectFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2015, Red Hat, Inc. and individual contributors + * Copyright 2016, Red Hat, Inc. and individual contributors * as indicated by the @author tags. See the copyright.txt file in the * distribution for a full listing of individual contributors. * @@ -48,14 +48,22 @@ public ClientResponse handle(ClientRequest clientRequest) Response.Status.Family.REDIRECTION) { return resp; } else { - // try location - log.debug("Server returns redirection status: {}. Try to follow it", - resp.getClientResponseStatus()); - URI redirectTarget = resp.getLocation(); - if (redirectTarget != null) { - clientRequest.setURI(redirectTarget); + // try location only if for GET and HEAD + String method = clientRequest.getMethod(); + if ("HEAD".equals(method) || "GET".equals(method)) { + log.debug( + "Server returns redirection status: {}. Try to follow it", + resp.getClientResponseStatus()); + URI redirectTarget = resp.getLocation(); + if (redirectTarget != null) { + clientRequest.setURI(redirectTarget); + } + return ch.handle(clientRequest); + } else { + throw new IllegalStateException( + "Received status " + resp.getClientResponseStatus() + + ". Check your server URL (e.g. used http instead of https)"); } - return ch.handle(clientRequest); } } } diff --git a/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java b/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java index 8b28d51e..63095d5d 100644 --- a/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java +++ b/zanata-rest-client/src/test/java/org/zanata/rest/client/InvalidContentTypeFilterTest.java @@ -3,11 +3,14 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.ws.rs.core.MediaType; + import org.hamcrest.MatcherAssert; import org.junit.Test; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; +import static org.zanata.rest.client.InvalidContentTypeFilter.isContentTypeCompatible; public class InvalidContentTypeFilterTest { @@ -26,7 +29,8 @@ public class InvalidContentTypeFilterTest { @Test public void testPatternMatch() { - Pattern pattern = Pattern.compile(".*(.*).*", Pattern.CASE_INSENSITIVE); + Pattern pattern = Pattern.compile(".*(.*).*", + Pattern.CASE_INSENSITIVE); Matcher matcher = pattern.matcher(sampleText.replaceAll("\\n", " ")); @@ -34,4 +38,36 @@ public void testPatternMatch() { MatcherAssert.assertThat(matcher.group(1), equalTo("Zanata: Home")); } + @Test + public void testValidateContentTypes() { + + MatcherAssert.assertThat( + isContentTypeCompatible(MediaType.TEXT_HTML_TYPE), + equalTo(false)); + MatcherAssert.assertThat( + isContentTypeCompatible( + MediaType.APPLICATION_FORM_URLENCODED_TYPE), + equalTo(false)); + MatcherAssert.assertThat( + isContentTypeCompatible(MediaType.MULTIPART_FORM_DATA_TYPE), + equalTo(false)); + + MatcherAssert.assertThat( + isContentTypeCompatible(MediaType.APPLICATION_XML_TYPE), + equalTo(true)); + MatcherAssert.assertThat( + isContentTypeCompatible(MediaType.APPLICATION_JSON_TYPE), + equalTo(true)); + MatcherAssert.assertThat( + isContentTypeCompatible(MediaType.WILDCARD_TYPE), + equalTo(true)); + MatcherAssert.assertThat(isContentTypeCompatible( + new MediaType("application", "vnd.zanata+xml")), equalTo(true)); + MatcherAssert.assertThat( + isContentTypeCompatible(MediaType.TEXT_PLAIN_TYPE), + equalTo(true)); + MatcherAssert.assertThat( + isContentTypeCompatible(null), + equalTo(true)); + } } From 4f683779d2a4e6fa9fdfe3b61e8a031ed60cf32f Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 3 Mar 2016 16:45:56 +1000 Subject: [PATCH 3/3] ZNTA-937 - only validate content type when status is successful --- .../org/zanata/rest/client/InvalidContentTypeFilter.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java b/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java index 2545a002..aa6da70d 100644 --- a/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java +++ b/zanata-rest-client/src/main/java/org/zanata/rest/client/InvalidContentTypeFilter.java @@ -24,6 +24,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,7 +60,9 @@ public ClientResponse handle(ClientRequest clientRequest) ClientHandler ch = getNext(); ClientResponse resp = ch.handle(clientRequest); - if (!isContentTypeCompatible(resp.getType())) { + if (resp.getClientResponseStatus().getFamily().equals( + Response.Status.Family.SUCCESSFUL) && + !isContentTypeCompatible(resp.getType())) { log.error(ERROR_MSG); String title = findPageTitle(resp); String snippet = String.format(