Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Use Jetty's ProxyServlet implementation #2753

Merged
merged 5 commits into from
Feb 13, 2017
Merged

Use Jetty's ProxyServlet implementation #2753

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
597fde5
Use Jetty's ProxyServlet implementation
watou Jan 4, 2017
019324b
Send specific errors back to client instead of logging them on the se…
watou Feb 7, 2017
3ce9d4d
Register Jetty async proxy servlet only if required servlet API is av…
watou Feb 10, 2017
eb93811
Rename impl classes to indicate intended differences
watou Feb 11, 2017
23485e6
Added optional resolution of org.eclipse.jetty.proxy package for TPs …
watou Feb 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bundles/ui/org.eclipse.smarthome.ui/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Import-Package: javax.imageio,
org.eclipse.jetty.client.api,
org.eclipse.jetty.client.util,
org.eclipse.jetty.http,
org.eclipse.jetty.proxy,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you add optional=true here, we would be fully backward compatible - i.e. the jetty-proxy bundle does not even have to be present, in which case we would automatically fall back to the old implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaikreuzer Just to be clear, are you suggesting that I add optional=true now, and that no other code changes are required for this to work in the way you described? Once org.eclipse.jetty.proxy is in the TP (which I think you are going to do), is optional=true needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to have it in the TP in any case, otherwise the code won't compile, so I will also have to add it to the OH2 IDE TP.
The "optional=true" is relevant at runtime: with it, the smarthome.ui bundle can be resolved and used even if jetty-proxy bundle is NOT available (and I guess on a system which only has servlet 2.4, it simply CANNOT be available anyhow).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added optional resolution of org.eclipse.jetty.proxy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect, thanks.

org.eclipse.jetty.util,
org.eclipse.jetty.util.component,
org.eclipse.jetty.util.ssl,
Expand Down
3 changes: 3 additions & 0 deletions bundles/ui/org.eclipse.smarthome.ui/OSGI-INF/proxy.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
-->
<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" activate="activate" deactivate="deactivate" immediate="true" name="org.eclipse.smarthome.ui.proxy">
<implementation class="org.eclipse.smarthome.ui.internal.proxy.ProxyServlet"/>

<property name="service.pid" type="String" value="org.eclipse.smarthome.proxy"/>

<reference bind="setItemUIRegistry" cardinality="1..1" interface="org.eclipse.smarthome.ui.items.ItemUIRegistry" name="ItemUIRegistry" policy="dynamic" unbind="unsetItemUIRegistry"/>
<reference bind="setHttpService" cardinality="1..1" interface="org.osgi.service.http.HttpService" name="HttpService" policy="dynamic" unbind="unsetHttpService"/>
<reference bind="setModelRepository" cardinality="1..1" interface="org.eclipse.smarthome.model.core.ModelRepository" name="ModelRepository" policy="static" unbind="unsetModelRepository"/>
Expand Down
213 changes: 97 additions & 116 deletions ...ipse.smarthome.ui/src/main/java/org/eclipse/smarthome/ui/internal/proxy/ProxyServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,18 @@
*/
package org.eclipse.smarthome.ui.internal.proxy;

import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.util.Hashtable;
import java.util.Iterator;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.Map;
import java.util.Objects;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.apache.commons.io.IOUtils;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.proxy.AsyncProxyServlet;
import org.eclipse.jetty.util.B64Code;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.ssl.SslContextFactory;
Expand Down Expand Up @@ -69,22 +60,19 @@
*
* @author Kai Kreuzer - Initial contribution and API
* @author Svilen Valkanov - Replaced Apache HttpClient with Jetty
* @author John Cocula - added optional Image/Video item= support
* @author John Cocula - added optional Image/Video item= support; refactor
*/
public class ProxyServlet extends HttpServlet {
public class ProxyServlet extends AsyncProxyServlet {

/** the alias for this servlet */
public static final String PROXY_ALIAS = "proxy";
private static final String CONFIG_MAX_THREADS = "maxThreads";
private static final int DEFAULT_MAX_THREADS = 8;
private static final String ATTR_URI = ProxyServlet.class.getName() + ".URI";

private final Logger logger = LoggerFactory.getLogger(ProxyServlet.class);

private static final long serialVersionUID = -4716754591953017793L;

private static HttpClient httpClient = new HttpClient(new SslContextFactory());

/** Timeout for HTTP requests in ms */
private static final int TIMEOUT = 15000;

protected HttpService httpService;
protected ItemUIRegistry itemUIRegistry;
protected ModelRepository modelRepository;
Expand Down Expand Up @@ -113,23 +101,45 @@ protected void unsetHttpService(HttpService httpService) {
this.httpService = null;
}

protected void activate() {
protected void activate(Map<String, Object> config) {
try {
logger.debug("Starting up proxy servlet at /{}", PROXY_ALIAS);

startHttpClient(httpClient);
Hashtable<String, String> props = new Hashtable<String, String>();
Hashtable<String, String> props = propsFromConfig(config);
httpService.registerServlet("/" + PROXY_ALIAS, this, props, createHttpContext());
} catch (NamespaceException e) {
logger.error("Error during servlet startup: {}", e.getMessage());
} catch (ServletException e) {
} catch (NamespaceException | ServletException e) {
logger.error("Error during servlet startup: {}", e.getMessage());
}
}

protected void deactivate() {
httpService.unregister("/" + PROXY_ALIAS);
stopHttpClient(httpClient);
try {
httpService.unregister("/" + PROXY_ALIAS);
} catch (IllegalArgumentException e) {
// ignore, had not been registered before
}
}

/**
* Copy the ConfigAdminManager's config to the init parameters of the servlet.
*
* @param config the OSGi config, may be <code>null</code>
* @return properties to pass to servlet for initialization
*/
private Hashtable<String, String> propsFromConfig(Map<String, Object> config) {
Hashtable<String, String> props = new Hashtable<String, String>();

if (config != null) {
for (String key : config.keySet()) {
props.put(key, config.get(key).toString());
}
}

// must specify, per http://stackoverflow.com/a/27625380
if (props.get(CONFIG_MAX_THREADS) == null) {
props.put(CONFIG_MAX_THREADS, String.valueOf(DEFAULT_MAX_THREADS));
}

return props;
}

/**
Expand All @@ -147,96 +157,85 @@ public String getServletInfo() {
return "Image and Video Widget Proxy";
}

/**
* {@inheritDoc}
*
* Override <code>newHttpClient</code> so we can proxy to HTTPS URIs.
*/
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {

String sitemapName = request.getParameter("sitemap");
String widgetId = request.getParameter("widgetId");
URI uri = getURI(sitemapName, widgetId, response);
protected HttpClient newHttpClient() {
return new HttpClient(new SslContextFactory());
}

if (uri != null) {
Request httpRequest = httpClient.newRequest(uri);
/**
* {@inheritDoc}
*
* Add Basic Authentication header to request if user and password are specified in URI.
* After Jetty is upgrade past 9.2.9, change to copyRequestHeaders to avoid deprecated warning.
*/
@SuppressWarnings("deprecation")
@Override
protected void copyHeaders(HttpServletRequest clientRequest, Request proxyRequest) {
super.copyHeaders(clientRequest, proxyRequest);

// check if the uri uses credentials and configure the http client accordingly
if (uri.getUserInfo() != null) {
String[] userInfo = uri.getUserInfo().split(":");
// check if the URI uses credentials and configure the HTTP client accordingly
URI uri = uriFromRequest(clientRequest);
if (uri != null && uri.getUserInfo() != null) {
String[] userInfo = uri.getUserInfo().split(":");

if (userInfo.length == 2) {
String user = userInfo[0];
String password = userInfo[1];

String basicAuthentication = "Basic " + B64Code.encode(user + ":" + password, StringUtil.__ISO_8859_1);
httpRequest.header(HttpHeader.AUTHORIZATION, basicAuthentication);
}

InputStreamResponseListener listener = new InputStreamResponseListener();

// do the client request
try {
httpRequest.send(listener);
// wait for the response headers to arrive or the timeout to expire
Response httpResponse = listener.get(TIMEOUT, TimeUnit.MILLISECONDS);

// get response headers
HttpFields headers = httpResponse.getHeaders();
Iterator<HttpField> iterator = headers.iterator();

// copy all headers
while (iterator.hasNext()) {
HttpField header = iterator.next();
response.setHeader(header.getName(), header.getValue());
}

} catch (Exception e) {
if (e instanceof TimeoutException) {
logger.warn("Proxy servlet failed to stream content due to a timeout");
response.sendError(HttpServletResponse.SC_GATEWAY_TIMEOUT);
} else {
logger.warn("Proxy servlet failed to stream content: {}", e.getMessage());
response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
}
return;
}
// now copy/stream the body content
try (InputStream responseContent = listener.getInputStream()) {
IOUtils.copy(responseContent, response.getOutputStream());
proxyRequest.header(HttpHeader.AUTHORIZATION, basicAuthentication);
}
}
}

/**
* Given a sitemap and widget, return the URI referenced by the widget in the sitemap.
* If the widget is not an Image or Video widget, then return <code>null</code>.
* If the widget is associated with an item, attempt to use the item's state as a URL.
* If the item's state as a string does not conform to RFC 2396, attempt to use the
* <code>url=</code> attribute in the sitemap. If that too does not conform to RFC 2396,
* then return <code>null</code>. In all cases where <code>null</code> is returned,
* this method first sends {@link HttpServletResponse.SC_BAD_REQUEST} back to the client.
* {@inheritDoc}
*/
@Override
protected String rewriteTarget(HttpServletRequest request) {
return Objects.toString(uriFromRequest(request), null);
}

/**
* Determine which URI to address based on the request contents.
*
* @param sitemapName
* @param widgetId
* @param response the HttpServletResponse to which detailed errors are sent
* @return the URI referenced by the widget
* @param request the servlet request
* @return the URI indicated by the request, or <code>null</code> if not possible
*/
private URI getURI(String sitemapName, String widgetId, HttpServletResponse response) throws IOException {
private URI uriFromRequest(HttpServletRequest request) {

// Return any URI we've already saved for this request
URI uri = (URI) request.getAttribute(ATTR_URI);
if (uri != null) {
return uri;
}

String sitemapName = request.getParameter("sitemap");
if (sitemapName == null) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Parameter 'sitemap' must be provided!");
logger.error("Parameter 'sitemap' must be provided!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not really like such things to be logged as errors in the runtime log. After all, these might be unsolicited requests from remote clients and errors should rather turn up in the client (if on the server, it should rather be some http access/error log).
Is there no way to send an HTTP error back here as it was done before?

return null;
}

String widgetId = request.getParameter("widgetId");
if (widgetId == null) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Parameter 'widget' must be provided!");
logger.error("Parameter 'widgetId' must be provided!");
return null;
}

Sitemap sitemap = (Sitemap) modelRepository.getModel(sitemapName);
if (sitemap == null) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Sitemap '" + sitemapName + "' could not be found!");
logger.error("Sitemap '{}' could not be found!", sitemapName);
return null;
}

Widget widget = itemUIRegistry.getWidget(sitemap, widgetId);
if (widget == null) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Widget '" + widgetId + "' could not be found!");
logger.error("Widget '{}' could not be found!", widgetId);
return null;
}

Expand All @@ -246,8 +245,7 @@ private URI getURI(String sitemapName, String widgetId, HttpServletResponse resp
} else if (widget instanceof Video) {
uriString = ((Video) widget).getUrl();
} else {
response.sendError(HttpServletResponse.SC_BAD_REQUEST,
"Widget type '" + widget.getClass().getName() + "' is not supported!");
logger.error("Widget type '{}' is not supported!", widget.getClass().getName());
return null;
}

Expand All @@ -256,39 +254,22 @@ private URI getURI(String sitemapName, String widgetId, HttpServletResponse resp
State state = itemUIRegistry.getItemState(itemName);
if (state != null && state instanceof StringType) {
try {
return URI.create(state.toString());
uri = URI.create(state.toString());
request.setAttribute(ATTR_URI, uri);
return uri;
} catch (IllegalArgumentException ex) {
// fall thru
}
}
}

try {
return URI.create(uriString);
uri = URI.create(uriString);
request.setAttribute(ATTR_URI, uri);
return uri;
} catch (IllegalArgumentException e) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST,
"URI '" + uriString + "' is not valid: " + e.getMessage());
logger.error("URI '{}' is not valid: {}", uriString, e.getMessage());
return null;
}
}

private void startHttpClient(HttpClient client) {
if (!client.isStarted()) {
try {
client.start();
} catch (Exception e) {
logger.warn("Cannot start HttpClient!", e);
}
}
}

private void stopHttpClient(HttpClient client) {
if (client.isStarted()) {
try {
client.stop();
} catch (Exception e) {
logger.error("Unable to stop HttpClient!", e);
}
}
}
}
Loading