Hi all,

This CL unfortunately suffers from a lot of static cling. There's no way to
customize this behavior, and the default behavior ends up having odd side
effects like getting and setting system properties, many of which are
irrelevant, and tightly monitored, in production environments.

I've temporarily pulled this logic out into a provider pattern:
http://codereview.appspot.com/4527056/

...so that users aren't forced into this behavior.

This said, I actually feel that the better solution for this is to put
host-substitution logic into ContainerConfig. All these config values come
from there anyway.

--j

On Thu, May 12, 2011 at 1:00 PM, <[email protected]> wrote:

> Author: woodser
> Date: Thu May 12 20:00:18 2011
> New Revision: 1102446
>
> URL: http://svn.apache.org/viewvc?rev=1102446&view=rev
> Log:
> Committing Li Xu's patch to detect host & port from HttpServletRequest.
>
> Codereview: http://codereview.appspot.com/4534049/
> JIRA: https://issues.apache.org/jira/browse/SHINDIG-1534
>
> Added:
>
>  
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/HostFilter.java
>
>  
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/ServletRequestContext.java
> Modified:
>    shindig/trunk/config/container.js
>
>  
> shindig/trunk/content/samplecontainer/examples/commoncontainer/gadgetCollections.json
>
>  
> shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js
>
>  
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java
>
>  
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
>
>  
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java
>
>  
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java
>    shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml
>
> Modified: shindig/trunk/config/container.js
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/config/container.js?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> --- shindig/trunk/config/container.js (original)
> +++ shindig/trunk/config/container.js Thu May 12 20:00:18 2011
> @@ -103,10 +103,13 @@
>  //"gadgets.securityTokenKeyFile" : "/path/to/key/file.txt",
>
>  // URI for the default shindig test instance.
> -"defaultShindigTestHost": "http://${SERVER_HOST}:${SERVER_PORT}";,
> +//"defaultShindigTestHost": "http://${SERVER_HOST}:${SERVER_PORT}";,
> +"defaultShindigTestHost":"http://%host%";,
> +
>
>  // Authority (host:port without scheme) for the proxy and concat servlets.
> -"defaultShindigProxyConcatAuthority": "${SERVER_HOST}:${SERVER_PORT}",
> +//"defaultShindigProxyConcatAuthority": "${SERVER_HOST}:${SERVER_PORT}",
> +"defaultShindigProxyConcatAuthority":"%host%",
>
>  // OS 2.0 Gadget DOCTYPE: used in Gadgets with @specificationVersion 2.0
> or greater and
>  // quirksmode on Gadget has not been set.
>
> Modified:
> shindig/trunk/content/samplecontainer/examples/commoncontainer/gadgetCollections.json
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/content/samplecontainer/examples/commoncontainer/gadgetCollections.json?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> ---
> shindig/trunk/content/samplecontainer/examples/commoncontainer/gadgetCollections.json
> (original)
> +++
> shindig/trunk/content/samplecontainer/examples/commoncontainer/gadgetCollections.json
> Thu May 12 20:00:18 2011
> @@ -4,22 +4,22 @@
>             "name": "Publish Subscribe Demo",
>             "Description": "This is a sample pub/sub demo showcasing the
> pubsub-2 feature",
>             "apps" : [
> -                     {"name": "publisher", "url": "
> http://localhost:8080/container/sample-pubsub-2-publisher.xml"},
> -                     {"name": "subscriber", "url": "
> http://localhost:8080/container/sample-pubsub-2-subscriber.xml"}
> +                     {"name": "publisher", "url":
> "/container/sample-pubsub-2-publisher.xml"},
> +                     {"name": "subscriber", "url":
> "/container/sample-pubsub-2-subscriber.xml"}
>                      ]
>              },
>              {
>             "name": "Activity Streams Sample",
>             "Description": "Simple gadget to test base ActivityStreams
> implementation in features",
>             "apps" : [
> -                     {"name": "ActivityStreams Sample", "url": "
> http://localhost:8080/samplecontainer/examples/ActivityStreams/ActivityStreamGadget.xml
> "}
> +                     {"name": "ActivityStreams Sample", "url":
> "/samplecontainer/examples/ActivityStreams/ActivityStreamGadget.xml"}
>                      ]
>              },
>                          {
>             "name": "Sample Media Items Gadget",
>             "Description": "This is a sample pub/sub demo showcasing the
> pubsub-2 feature",
>             "apps" : [
> -                     {"name": "publisher", "url": "
> http://localhost:8080/samplecontainer/examples/media/Media.xml"}
> +                     {"name": "publisher", "url":
> "/samplecontainer/examples/media/Media.xml"}
>                      ]
>              },
>              {
>
> Modified:
> shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> ---
> shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js
> (original)
> +++
> shindig/trunk/content/samplecontainer/examples/commoncontainer/viewController.js
> Thu May 12 20:00:18 2011
> @@ -45,7 +45,11 @@ $(function() {
>                          $.each(data.collections, function(i,data){
>                                 var optionVal = [];
>                                 $.each(data.apps, function(i,data){
> -                                     optionVal.push(data.url);
> +                                  if (data.url.indexOf("http") < 0){
> +
>  optionVal.push(location.protocol+"//"+location.host+data.url);
> +                                  }else{
> +                                        optionVal.push(data.url);
> +                                  }
>                                 });
>                             $('#gadgetCollection').append('<option
> value="'+ optionVal.toString() + '">' + data.name +'</option>');
>                           });
>
> Added:
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/HostFilter.java
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/HostFilter.java?rev=1102446&view=auto
>
> ==============================================================================
> ---
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/HostFilter.java
> (added)
> +++
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/HostFilter.java
> Thu May 12 20:00:18 2011
> @@ -0,0 +1,50 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +
> +package org.apache.shindig.common.servlet;
> +
> +import java.io.IOException;
> +
> +import javax.servlet.Filter;
> +import javax.servlet.FilterConfig;
> +import javax.servlet.FilterChain;
> +import javax.servlet.ServletContext;
> +import javax.servlet.ServletException;
> +import javax.servlet.ServletRequest;
> +import javax.servlet.ServletResponse;
> +
> +
> +
> +
> +/**
> + * A Filter that can cache ServletRequest information in ThreadLocal
> variable
> + */
> +public class HostFilter implements Filter {
> +
> +  public void doFilter(ServletRequest request, ServletResponse response,
> FilterChain chain) throws IOException, ServletException {
> +    ServletRequestContext.setRequestInfo(request);
> +    chain.doFilter(request, response);
> +  }
> +
> +  public void destroy() {
> +  }
> +
> +  public void init(FilterConfig arg0) throws ServletException {
> +  }
> +}
>
> Added:
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/ServletRequestContext.java
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/ServletRequestContext.java?rev=1102446&view=auto
>
> ==============================================================================
> ---
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/ServletRequestContext.java
> (added)
> +++
> shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/ServletRequestContext.java
> Thu May 12 20:00:18 2011
> @@ -0,0 +1,105 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.apache.shindig.common.servlet;
> +
> +import javax.servlet.ServletRequest;
> +
> +public class ServletRequestContext {
> +
> +  public static void setRequestInfo(ServletRequest req) {
> +    String auth = req.getServerName() + ":" + req.getServerPort();
> +       String fullAuth = req.getScheme() + "://" + auth;
> +       authority.set(auth);
> +       fullAuthority.set(fullAuth);
> +
> +       System.setProperty("authority",auth);
> +       System.setProperty("fullAuthority", fullAuth);
> +  }
> +
> +  /**
> +   * A Thread Local holder for authority -- host + port
> +   */
> +  private static ThreadLocal<String> authority = new
> ThreadLocal<String>();
> +
> +  /**
> +   * A Thread Local holder for full authority -- scheme + host + port
> +   */
> +  private static ThreadLocal<String> fullAuthority = new
> ThreadLocal<String>();
> +
> +
> +  public static String getAuthority() {
> +
> +    String retVal = authority.get();
> +    if (retVal == null) {
> +      retVal = System.getProperty("authority");
> +      if (retVal == null){
> +       retVal = getDefaultAuthority();
> +      }
> +    }
> +    return retVal;
> +  }
> +
> +  private static String getDefaultAuthority() {
> +
> +    String retVal = System.getProperty("defaultAuthority");
> +       if (retVal == null){
> +         retVal = getServerHostname()+":"+getServerPort();
> +         System.setProperty("defaultAuthority", retVal);
> +       }
> +       return retVal;
> +
> +  }
> +
> +  public static String getFullAuthority() {
> +
> +       String retVal = fullAuthority.get();
> +    if (retVal == null) {
> +      retVal = System.getProperty("fullAuthority");
> +      if (retVal == null){
> +        retVal = getDefaultFullAuthority();
> +         }
> +       }
> +    return retVal;
> +
> +  }
> +
> +  private static String getDefaultFullAuthority() {
> +
> +    String retVal = System.getProperty("defaultFullAuthority");
> +    if ( retVal != null ){
> +         retVal = "http://"+getServerHostname()+":"+getServerPort();
> +         System.setProperty("defaultFullAuthority", retVal);
> +       }
> +       return retVal;
> +
> +  }
> +
> +  private static String getServerPort() {
> +       return System.getProperty("shindig.port") != null ?
> System.getProperty("shindig.port") :
> +         System.getProperty("jetty.port") != null ?
> System.getProperty("jetty.port") :
> +         "8080";
> +  }
> +
> +  private static String getServerHostname() {
> +       return System.getProperty("shindig.host") != null ?
> System.getProperty("shindig.host") :
> +         System.getProperty("jetty.host") != null ?
> System.getProperty("jetty.host") :
> +         "localhost";
> +  }
> +
> +}
>
> Modified:
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> ---
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java
> (original)
> +++
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java
> Thu May 12 20:00:18 2011
> @@ -24,6 +24,7 @@ import com.google.inject.Inject;
>  import com.google.inject.name.Named;
>
>  import org.apache.commons.lang.StringEscapeUtils;
> +import org.apache.shindig.common.servlet.ServletRequestContext;
>  import org.apache.shindig.common.uri.Uri;
>  import org.apache.shindig.common.uri.UriBuilder;
>  import org.apache.shindig.config.ContainerConfig;
> @@ -222,6 +223,8 @@ public class DefaultConcatUriManager imp
>       throw new RuntimeException(
>           "Missing required config '" + key + "' for container: " +
> container);
>     }
> +    val = val.replace("%host%", ServletRequestContext.getAuthority());
> +
>     return val;
>   }
>
>
> Modified:
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> ---
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
> (original)
> +++
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
> Thu May 12 20:00:18 2011
> @@ -27,6 +27,7 @@ import org.apache.commons.lang.StringUti
>  import org.apache.shindig.auth.SecurityToken;
>  import org.apache.shindig.auth.SecurityTokenCodec;
>  import org.apache.shindig.auth.SecurityTokenException;
> +import org.apache.shindig.common.servlet.ServletRequestContext;
>  import org.apache.shindig.common.uri.Uri;
>  import org.apache.shindig.common.uri.UriBuilder;
>  import org.apache.shindig.config.ContainerConfig;
> @@ -342,10 +343,13 @@ public class DefaultIframeUriManager imp
>
>   private String getReqVal(String container, String key) {
>     String val = config.getString(container, key);
> +
>     if (val == null) {
>       throw new RuntimeException("Missing required container config param,
> key: "
>           + key + ", container: " + container);
>     }
> +    val = val.replace("%host%", ServletRequestContext.getAuthority());
> +
>     return val;
>   }
>
>
> Modified:
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> ---
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java
> (original)
> +++
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultJsUriManager.java
> Thu May 12 20:00:18 2011
> @@ -23,6 +23,7 @@ import com.google.common.base.Splitter;
>  import com.google.common.collect.ImmutableList;
>  import com.google.inject.Inject;
>
> +import org.apache.shindig.common.servlet.ServletRequestContext;
>  import org.apache.shindig.common.uri.Uri;
>  import org.apache.shindig.common.uri.UriBuilder;
>  import org.apache.shindig.config.ContainerConfig;
> @@ -222,6 +223,7 @@ public class DefaultJsUriManager impleme
>             "' missing config for required param: " + key);
>       }
>     }
> +    ret = ret.replace("%host%", ServletRequestContext.getAuthority());
>     return ret;
>   }
>
>
> Modified:
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> ---
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java
> (original)
> +++
> shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java
> Thu May 12 20:00:18 2011
> @@ -25,6 +25,7 @@ import com.google.inject.Inject;
>  import com.google.inject.name.Named;
>
>  import org.apache.commons.lang.StringUtils;
> +import org.apache.shindig.common.servlet.ServletRequestContext;
>  import org.apache.shindig.common.uri.Uri;
>  import org.apache.shindig.common.uri.UriBuilder;
>  import org.apache.shindig.common.util.Utf8UrlCoder;
> @@ -276,6 +277,7 @@ public class DefaultProxyUriManager impl
>       throw new RuntimeException("Missing required container config key: "
> + key + " for " +
>           "container: " + container);
>     }
> +    val = val.replace("%host%", ServletRequestContext.getAuthority());
>     return val;
>   }
>  }
>
> Modified: shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml
> URL:
> http://svn.apache.org/viewvc/shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml?rev=1102446&r1=1102445&r2=1102446&view=diff
>
> ==============================================================================
> --- shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml (original)
> +++ shindig/trunk/java/server/src/main/webapp/WEB-INF/web.xml Thu May 12
> 20:00:18 2011
> @@ -59,14 +59,24 @@
>    <context-param>
>        <param-name>system.properties</param-name>
>      <param-value>
> -       shindig.host=localhost
> -
> -       aKey=/shindig/gadgets/proxy?container=default&amp;url=
> -       shindig.port=
> -
> +       aKey=/shindig/gadgets/proxy?container=default&amp;url=
>      </param-value>
>   </context-param>
> -
> +
> +  <filter>
> +    <filter-name>hostFilter</filter-name>
> +
>  <filter-class>org.apache.shindig.common.servlet.HostFilter</filter-class>
> +  </filter>
> +  <filter-mapping>
> +    <filter-name>hostFilter</filter-name>
> +    <url-pattern>/gadgets/ifr</url-pattern>
> +    <url-pattern>/gadgets/js/*</url-pattern>
> +    <url-pattern>/gadgets/proxy/*</url-pattern>
> +    <url-pattern>/gadgets/concat</url-pattern>
> +    <url-pattern>/rpc/*</url-pattern>
> +    <url-pattern>/rest/*</url-pattern>
> +  </filter-mapping>
> +
>     <filter>
>         <filter-name>ShiroFilter</filter-name>
>
> <filter-class>org.apache.shiro.web.servlet.IniShiroFilter</filter-class>
>
>
>

Reply via email to