Author: etnu
Date: Thu Feb 21 00:00:16 2008
New Revision: 629709

URL: http://svn.apache.org/viewvc?rev=629709&view=rev
Log:
It turns out that many sites have more than one sub domain, and I wasn't smart 
enough to realize this, so I re-introduced the parent parameter in a way that 
is both compatible with existing sites and secure. You may now use a regular 
expression to validate that the parent parameter is acceptable for the given 
syndicator, and gadgets.rpc will append your parentRelayUrl to the "parent" 
parameter. Since the server is validating the parent, this can be trusted.

Also found and fixed the cause of of SHINDIG-43 in the process. The problem 
turned out to be a slightly different hashing algorithm that was changing the 
order of dependencies in the set, and our code didn't take that into account.


Modified:
    incubator/shindig/trunk/config/syndicator.js
    incubator/shindig/trunk/features/caja/feature.xml
    incubator/shindig/trunk/features/core.io/feature.xml
    incubator/shindig/trunk/features/flash/feature.xml
    incubator/shindig/trunk/features/minimessage/feature.xml
    incubator/shindig/trunk/features/opensocial-reference/feature.xml
    incubator/shindig/trunk/features/rpc/feature.xml
    incubator/shindig/trunk/features/rpc/rpc.js
    incubator/shindig/trunk/features/skins/feature.xml
    incubator/shindig/trunk/features/tabs/feature.xml
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java

Modified: incubator/shindig/trunk/config/syndicator.js
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/config/syndicator.js?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/config/syndicator.js (original)
+++ incubator/shindig/trunk/config/syndicator.js Thu Feb 21 00:00:16 2008
@@ -38,6 +38,13 @@
 // Location of opensocial-0.7 javascript (loaded server-side).
 "opensocial.0.7.location" : null, // not supported by default. over ride this 
in
                                   // your own syndicator file.
+// Set of regular expressions to validate the parent parameter. This is
+// necessary to support situations where you want a single syndicator to 
support
+// multiple possible host names (such as for localized domains, such as
+// <language>.example.org. If left as null, the parent parameter will be
+// ignored; otherwise, any requests that do not include a parent
+// value matching this set will return a 404 error.
+"gadgets.parent" : null,
 
 // This config data will be passed down to javascript. Please
 // configure your object using the feature name rather than
@@ -47,7 +54,8 @@
 // See individual feature.xml files for configuration details.
 "gadgets.features" : {
   "core.io" : {
-    "proxyUrl" : "http://www.gmodules.com/ig/proxy?url=%url%";,
+       // Note: /proxy is an open proxy. Be careful how you explose this!
+    "proxyUrl" : "proxy?url=%url%",
     "jsonProxyUrl" : "proxy?output=js",
   },
   "views" : {
@@ -64,9 +72,14 @@
     },
   },
   "rpc" : {
+       // Path to the relay file. Automatically appended to the parent
+       // parameter if it passes input validation and is not null.
     // This should never be on the same host in a production environment!
     // Only use this for TESTING!
     "parentRelayUrl" : "files/container/rpc_relay.html",
+
+    // If true, this will use the legacy ifpc wire format when making rpc
+    // requests.
     "useLegacyProtocol" : false,
   },
 }}

Modified: incubator/shindig/trunk/features/caja/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/caja/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/caja/feature.xml (original)
+++ incubator/shindig/trunk/features/caja/feature.xml Thu Feb 21 00:00:16 2008
@@ -21,7 +21,6 @@
 -->
 <feature>
   <name>caja</name>
-  <dependency>core</dependency>
   <gadget>
     <script src="caja.js"></script>
     <script src="html-sanitizer.js"></script>

Modified: incubator/shindig/trunk/features/core.io/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/core.io/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/core.io/feature.xml (original)
+++ incubator/shindig/trunk/features/core.io/feature.xml Thu Feb 21 00:00:16 
2008
@@ -28,6 +28,9 @@
       encoded inside of the POST body.
 -->
   <name>core.io</name>
+       <!-- core.io needs an explicit core dependency to ensure that the graph
+           is resolved in the correct order.
+       -->
   <dependency>core</dependency>
   <gadget>
     <script src="io.js"/>

Modified: incubator/shindig/trunk/features/flash/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/flash/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/flash/feature.xml (original)
+++ incubator/shindig/trunk/features/flash/feature.xml Thu Feb 21 00:00:16 2008
@@ -18,7 +18,6 @@
 -->
 <feature>
   <name>flash</name>
-  <dependency>core</dependency>
   <gadget>
     <script src="flash.js"/>
   </gadget>

Modified: incubator/shindig/trunk/features/minimessage/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/minimessage/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/minimessage/feature.xml (original)
+++ incubator/shindig/trunk/features/minimessage/feature.xml Thu Feb 21 
00:00:16 2008
@@ -18,7 +18,6 @@
 -->
 <feature>
   <name>minimessage</name>
-  <dependency>core</dependency>
   <gadget>
     <script src="minimessage.js"/>
   </gadget>

Modified: incubator/shindig/trunk/features/opensocial-reference/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/opensocial-reference/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/opensocial-reference/feature.xml (original)
+++ incubator/shindig/trunk/features/opensocial-reference/feature.xml Thu Feb 
21 00:00:16 2008
@@ -20,7 +20,7 @@
 <feature>
   <name>opensocial-reference</name>
        <!-- all known opensocial implementations assume that ifpc is present. 
-->
-       <dependency>rpc</dependency>
+       <dependency>ifpc</dependency>
   <gadget>
     <script src="opensocial.js"/>
     <script src="activity.js"/>

Modified: incubator/shindig/trunk/features/rpc/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/rpc/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/rpc/feature.xml (original)
+++ incubator/shindig/trunk/features/rpc/feature.xml Thu Feb 21 00:00:16 2008
@@ -28,7 +28,6 @@
 (if true, all calls to rpc.call will use the same wire format as ifpc).
 -->
   <name>rpc</name>
-  <dependency>core</dependency>
   <gadget>
     <script src="rpc.js"/>
   </gadget>

Modified: incubator/shindig/trunk/features/rpc/rpc.js
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/rpc/rpc.js?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/rpc/rpc.js (original)
+++ incubator/shindig/trunk/features/rpc/rpc.js Thu Feb 21 00:00:16 2008
@@ -35,6 +35,7 @@
   var useLegacyProtocol = {};
   var callId = 0;
   var callbacks = {};
+  var parentUrl = gadgets.util.getUrlParameters().parent || '';
 
   // Pick the most efficient RPC relay mechanism
   var relayChannel = typeof document.postMessage === 'function' ? 'dpm' :
@@ -141,7 +142,13 @@
      * Initializes RPC from the provided configuration.
      */
     function init(config) {
-      relayUrl['..'] = config.rpc.parentRelayUrl;
+      // Allow for wild card parent relay files as long as it's from a
+      // white listed domain. This is enforced by the rendering servlet.
+      if (config.rpc.parentRelayUrl.substring(0, 7) === 'http://') {
+        relayUrl['..'] = config.rpc.parentRelayUrl;
+      } else {
+        relayUrl['..'] = parentUrl + config.rpc.parentRelayUrl;
+      }
       useLegacyProtocol['..'] = !!config.rpc.useLegacyProtocol;
     }
 

Modified: incubator/shindig/trunk/features/skins/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/skins/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/skins/feature.xml (original)
+++ incubator/shindig/trunk/features/skins/feature.xml Thu Feb 21 00:00:16 2008
@@ -18,7 +18,6 @@
 -->
 <feature>
   <name>skins</name>
-  <dependency>core</dependency>
   <gadget>
     <script src="skins.js"/>
   </gadget>

Modified: incubator/shindig/trunk/features/tabs/feature.xml
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/features/tabs/feature.xml?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- incubator/shindig/trunk/features/tabs/feature.xml (original)
+++ incubator/shindig/trunk/features/tabs/feature.xml Thu Feb 21 00:00:16 2008
@@ -18,7 +18,6 @@
 -->
 <feature>
   <name>tabs</name>
-  <dependency>core</dependency>
   <gadget>
     <script src="tabs.js"/>
   </gadget>

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java
 Thu Feb 21 00:00:16 2008
@@ -78,16 +78,18 @@
 
     List<String> coreDeps = new LinkedList<String>();
     JsFeatureLoader loader = new JsFeatureLoader();
-    List<Entry> jsFeatures = loader.loadFeatures(featurePath, this);
+    Set<Entry> jsFeatures = loader.loadFeatures(featurePath, this);
 
     if (!coreDone) {
       for (Entry entry : jsFeatures) {
-        if (entry.name.startsWith("core")) {
+        if (entry.name.startsWith("core") || entry.name.equals("core")) {
           coreDeps.add(entry.getName());
           core.put(entry.getName(), entry);
         }
       }
 
+      logger.info("Core dependencies: " + coreDeps);
+
       // Everything depends on core JS being set up first because in gadget
       // rendering mode, we pre-populate some of the data.
       core.put(FEAT_MSG_BUNDLE,
@@ -100,7 +102,7 @@
 
       // Make sure non-core features depend on core.
       for (Entry entry : jsFeatures) {
-        if (!entry.name.startsWith("core")) {
+        if (!entry.name.startsWith("core") && !entry.name.equals("core")) {
           entry.deps.addAll(core.values());
         }
       }
@@ -275,6 +277,8 @@
       if (rhs instanceof Entry) {
         Entry entry = (Entry)rhs;
         return name.equals(entry.name);
+      } else if (rhs instanceof String) {
+        return name.equals(rhs);
       }
       return false;
     }

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java
 Thu Feb 21 00:00:16 2008
@@ -28,6 +28,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
@@ -70,7 +71,7 @@
    * @return A list of the newly loaded features.
    * @throws GadgetException
    */
-  public List<GadgetFeatureRegistry.Entry> loadFeatures(String path,
+  public Set<GadgetFeatureRegistry.Entry> loadFeatures(String path,
       GadgetFeatureRegistry registry) throws GadgetException {
     Map<String, ParsedFeature> deps = new HashMap<String, ParsedFeature>();
     try {
@@ -91,20 +92,16 @@
       throw new GadgetException(GadgetException.Code.INVALID_PATH, e);
     }
 
-    List<GadgetFeatureRegistry.Entry> entries
-        = new LinkedList<GadgetFeatureRegistry.Entry>();
+
 
     // This ensures that we register everything in the right order.
-    Set<String> registered = new HashSet<String>();
+    Set<GadgetFeatureRegistry.Entry> registered
+        = new HashSet<GadgetFeatureRegistry.Entry>();
     for (Map.Entry<String, ParsedFeature> entry : deps.entrySet()) {
       ParsedFeature feature = entry.getValue();
-      GadgetFeatureRegistry.Entry feat
-          = register(registry, feature, registered, deps);
-      if (feat != null) {
-        entries.add(feat);
-      }
+      register(registry, feature, registered, deps);
     }
-    return entries;
+    return Collections.unmodifiableSet(registered);
   }
 
   /**
@@ -142,6 +139,8 @@
         ParsedFeature feature = parse(content, parent, true);
         if (feature != null) {
           feats.put(feature.name, feature);
+        } else {
+          logger.warning("Failed to parse feature: " + file);
         }
       }
     } catch (IOException e) {
@@ -186,24 +185,21 @@
    * @param registered Set of all features registered during this operation.
    * @param all Map of all features that can be loaded during this operation.
    */
-  private GadgetFeatureRegistry.Entry register(GadgetFeatureRegistry registry,
-                                               ParsedFeature feature,
-                                               Set<String> registered,
-                                               Map<String, ParsedFeature> all) 
{
-    if (registered.contains(feature.name)) {
-      return null;
-    }
-
-    for (String dep : feature.deps) {
-      if (all.containsKey(dep) && !registered.contains(dep)) {
-        register(registry, all.get(dep), registered, all);
+  private void register(GadgetFeatureRegistry registry,
+                        ParsedFeature feature,
+                        Set<GadgetFeatureRegistry.Entry> registered,
+                        Map<String, ParsedFeature> all) {
+    if (!registered.contains(feature.name)) {
+      for (String dep : feature.deps) {
+        if (all.containsKey(dep) && !registered.contains(dep)) {
+          register(registry, all.get(dep), registered, all);
+        }
       }
-    }
 
-    JsLibraryFeatureFactory factory
-        = new JsLibraryFeatureFactory(feature.gadgetJs, feature.containerJs);
-    registered.add(feature.name);
-    return registry.register(feature.name, feature.deps, factory);
+      JsLibraryFeatureFactory factory
+          = new JsLibraryFeatureFactory(feature.gadgetJs, feature.containerJs);
+      registered.add(registry.register(feature.name, feature.deps, factory));
+    }
   }
 
   /**
@@ -237,7 +233,8 @@
 
     NodeList nameNode = doc.getElementsByTagName("name");
     if (nameNode.getLength() != 1) {
-      throw new GadgetException(GadgetException.Code.MALFORMED_XML_DOCUMENT);
+      throw new GadgetException(GadgetException.Code.MALFORMED_XML_DOCUMENT,
+          "No name provided");
     }
     feature.name = nameNode.item(0).getTextContent();
 

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java?rev=629709&r1=629708&r2=629709&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java
 Thu Feb 21 00:00:16 2008
@@ -30,6 +30,7 @@
 import org.apache.shindig.gadgets.SyndicatorConfig;
 import org.apache.shindig.gadgets.UserPrefs;
 import org.apache.shindig.gadgets.GadgetFeatureRegistry.Entry;
+import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -47,6 +48,7 @@
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
@@ -65,7 +67,6 @@
   private static final Logger logger
       = Logger.getLogger("org.apache.shindig.gadgets");
 
-
   @Override
   public void init(ServletConfig config) throws ServletException {
     servletState = CrossServletState.get(config);
@@ -74,6 +75,7 @@
   @Override
   protected void doGet(HttpServletRequest req, HttpServletResponse resp)
       throws IOException {
+
     String urlParam = req.getParameter("url");
     if (urlParam == null) {
       resp.sendError(HttpServletResponse.SC_BAD_REQUEST,
@@ -92,15 +94,22 @@
       return;
     }
 
+    if (!validateParent(req)) {
+      logger.info("Invalid parent");
+      resp.sendError(HttpServletResponse.SC_BAD_REQUEST,
+          "Unsupported parent parameter. Check your syndicator code.");
+      return;
+    }
+
     int moduleId = 0;
     String mid = req.getParameter("mid");
     if (mid != null) {
       moduleId = Integer.parseInt(mid);
     }
 
+    ProcessingOptions options = new HttpProcessingOptions(req);
     BasicHttpContext context = new BasicHttpContext(req);
     GadgetView.ID gadgetId = new Gadget.GadgetId(uri, moduleId);
-    ProcessingOptions options = new HttpProcessingOptions(req);
 
     // Prepare a list of GadgetContentFilters applied to the output
     List<GadgetContentFilter> contentFilters =
@@ -398,5 +407,50 @@
         throw new RuntimeException(e);
       }
     }
+  }
+
+  /**
+   * Validates that the parent parameter was acceptable.
+   *
+   * @param request
+   * @return True if the parent parameter is valid for the current
+   *     syndicator.
+   */
+  private boolean validateParent(HttpServletRequest request) {
+    String syndicator = request.getParameter("synd");
+    if (syndicator == null) {
+      syndicator = SyndicatorConfig.DEFAULT_SYNDICATOR;
+    }
+
+    String parent = request.getParameter("parent");
+
+    if (parent == null) {
+      // If there is no parent parameter, we are still safe because no
+      // dependent code ever has to trust it anyway.
+      return true;
+    }
+
+    SyndicatorConfig syndConf
+        = servletState.getGadgetServer().getConfig().getSyndicatorConfig();
+
+    try {
+      JSONArray parents = syndConf.getJsonArray(syndicator, "gadgets.parent");
+
+      if (parents == null) {
+        return true;
+      } else {
+        // We need to check each possible parent parameter against this regex.
+        for (int i = 0, j = parents.length(); i < j; ++i) {
+          // TODO: Should patterns be cached? Recompiling every request
+          // seems wasteful.
+          if (Pattern.matches(parents.getString(i), parent)) {
+            return true;
+          }
+        }
+      }
+    } catch (JSONException e) {
+      logger.log(Level.WARNING, "Configuration error", e);
+    }
+    return false;
   }
 }


Reply via email to