You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2010/08/11 20:46:51 UTC
svn commit: r984526 - in /shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/rewrite/
test/java/org/apache/shindig/gadgets/rewrite/
Author: johnh
Date: Wed Aug 11 18:46:51 2010
New Revision: 984526
URL: http://svn.apache.org/viewvc?rev=984526&view=rev
Log:
Only the LINK tags which have rel="stylesheet" and type="text/css" should be
rewritten. Others should be bypassed.
There can be resources, which have the resource url as empty string; those
should be bypassed as well.
Patch provided by Kuntal Loya.
,--This line, and those below, will be ignored--
M java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitorTest.java
M java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java
M java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java
M java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitorTest.java
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java?rev=984526&r1=984525&r2=984526&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitor.java Wed Aug 11 18:46:51 2010
@@ -31,6 +31,7 @@ import org.w3c.dom.Document;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
+import org.w3c.dom.Element;
import java.util.HashMap;
import java.util.List;
@@ -120,6 +121,14 @@ public class AbsolutePathReferenceVisito
String nodeName = node.getNodeName().toLowerCase();
if (node.getNodeType() == Node.ELEMENT_NODE &&
resourceTags.containsKey(nodeName)) {
+ if (nodeName.equals("link")) {
+ // Rewrite link only when it is for css.
+ String type = ((Element)node).getAttribute("type");
+ String rel = ((Element)node).getAttribute("rel");
+ if (!"stylesheet".equalsIgnoreCase(rel) || !"text/css".equalsIgnoreCase(type)) {
+ return null;
+ }
+ }
Attr attr = (Attr) node.getAttributes().getNamedItem(
resourceTags.get(nodeName));
String nodeUri = attr != null ? attr.getValue() : null;
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java?rev=984526&r1=984525&r2=984526&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java Wed Aug 11 18:46:51 2010
@@ -20,6 +20,7 @@ package org.apache.shindig.gadgets.rewri
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
+import org.apache.commons.lang.StringUtils;
import org.apache.shindig.common.Pair;
import org.apache.shindig.common.uri.Uri;
import org.apache.shindig.common.uri.Uri.UriException;
@@ -65,11 +66,20 @@ public class ProxyingVisitor implements
if (node.getNodeType() == Node.ELEMENT_NODE &&
RESOURCE_TAGS.containsKey(nodeName) &&
featureConfig.shouldRewriteTag(nodeName)) {
+ if (nodeName.equals("link")) {
+ // Rewrite link only when it is for css.
+ String type = ((Element)node).getAttribute("type");
+ String rel = ((Element)node).getAttribute("rel");
+ if (!"stylesheet".equalsIgnoreCase(rel) || !"text/css".equalsIgnoreCase(type)) {
+ return VisitStatus.BYPASS;
+ }
+ }
+
Attr attr = (Attr)node.getAttributes().getNamedItem(
RESOURCE_TAGS.get(nodeName));
if (attr != null) {
String urlValue = attr.getValue();
- if (featureConfig.shouldRewriteURL(urlValue)) {
+ if (!StringUtils.isEmpty(urlValue) && featureConfig.shouldRewriteURL(urlValue)) {
return VisitStatus.RESERVE_NODE;
}
}
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java?rev=984526&r1=984525&r2=984526&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/AbsolutePathReferenceVisitorTest.java Wed Aug 11 18:46:51 2010
@@ -110,7 +110,32 @@ public class AbsolutePathReferenceVisito
@Test
public void absolutifyTagLink() throws Exception {
- checkAbsolutifyStates("link");
+ Element cssLink = elem("link", "href", RELATIVE_URI.toString(),
+ "rel", "stylesheet", "type", "text/css");
+ assertEquals("CSS link tag should not be bypassed",
+ VisitStatus.MODIFY, getVisitStatus(cssLink));
+ }
+
+ @Test
+ public void bypassTagLinkWithNoRel() throws Exception {
+ Element cssLink = elem("link", "href", RELATIVE_URI.toString(), "type", "text/css");
+ assertEquals("CSS link tag should be bypassed",
+ VisitStatus.BYPASS, getVisitStatus(cssLink));
+ }
+
+ @Test
+ public void bypassTagLinkWithNoType() throws Exception {
+ Element cssLink = elem("link", "href", RELATIVE_URI.toString(), "rel", "stylesheet");
+ assertEquals("CSS link tag should be bypassed",
+ VisitStatus.BYPASS, getVisitStatus(cssLink));
+ }
+
+ @Test
+ public void bypassTagLinkAlternate() throws Exception {
+ Element cssLink = elem("link", "href", RELATIVE_URI.toString(),
+ "rel", "alternate", "hreflang", "el");
+ assertEquals("CSS link tag should be bypassed",
+ VisitStatus.BYPASS, getVisitStatus(cssLink));
}
@Test
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitorTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitorTest.java?rev=984526&r1=984525&r2=984526&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitorTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitorTest.java Wed Aug 11 18:46:51 2010
@@ -17,8 +17,18 @@
*/
package org.apache.shindig.gadgets.rewrite;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
+import static org.easymock.EasyMock.capture;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
import org.apache.shindig.common.uri.Uri;
import org.apache.shindig.gadgets.Gadget;
import org.apache.shindig.gadgets.rewrite.DomWalker.Visitor.VisitStatus;
@@ -28,10 +38,11 @@ import org.junit.Test;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
-import java.util.List;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
-import static org.easymock.EasyMock.*;
-import static org.junit.Assert.*;
+import java.util.List;
+import java.util.ArrayList;
/**
* Test of proxying rewriter
@@ -60,10 +71,25 @@ public class ProxyingVisitorTest extends
}
@Test
- public void linkVisitReserved() throws Exception {
- checkVisitReserved("link", true);
+ public void csslinkVisitReserved() throws Exception {
+ checkVisitReserved("link", true, "rel", "stylesheet", "type", "text/css");
}
-
+
+ @Test
+ public void linkWithNoRelVisitReserved() throws Exception {
+ checkVisitReserved("link", false, "type", "text/css");
+ }
+
+ @Test
+ public void linkWithNoTypeVisitReserved() throws Exception {
+ checkVisitReserved("link", false, "rel", "stylesheet");
+ }
+
+ @Test
+ public void altlinkVisitReserved() throws Exception {
+ checkVisitReserved("link", false, "rel", "alternate", "hreflang", "el");
+ }
+
@Test
public void scriptVisitReserved() throws Exception {
checkVisitReserved("script", true);
@@ -78,30 +104,50 @@ public class ProxyingVisitorTest extends
public void otherVisitNotReserved() throws Exception {
checkVisitReserved("other", false);
}
-
- private void checkVisitReserved(String tag, boolean result) throws Exception {
+
+ @Test
+ public void imgWithEmptySrc() throws Exception {
+ Node node = elem("img", "src", "");
+ ContentRewriterFeature.Config config = createMock(ContentRewriterFeature.Config.class);
+ expect(config.shouldRewriteURL("")).andReturn(true).anyTimes();
+ expect(config.shouldRewriteTag("img")).andReturn(true).anyTimes();
+ replay(config);
+
+ ProxyingVisitor rewriter = new ProxyingVisitor(config, null);
+ VisitStatus status = rewriter.visit(null, node);
+ verify(config);
+
+ assertEquals("Empty attribute should not be rewritten", VisitStatus.BYPASS, status);
+ }
+
+ private void checkVisitReserved(String tag, boolean result, String ... attrs) throws Exception {
tag = tag.toLowerCase();
- assertEquals(result, ProxyingVisitor.RESOURCE_TAGS.containsKey(tag));
- assertEquals(result, getVisitReserved(tag, true, true));
- assertEquals(result, getVisitReserved(tag.toUpperCase(), true, true));
- assertFalse(getVisitReserved(tag, false, true));
- assertFalse(getVisitReserved(tag, true, false));
- assertFalse(getVisitReserved(tag, false, false));
+ assertEquals(result, getVisitReserved(tag, true, true, attrs));
+ assertEquals(result, getVisitReserved(tag.toUpperCase(), true, true, attrs));
+ assertFalse(getVisitReserved(tag, false, true, attrs));
+ assertFalse(getVisitReserved(tag, true, false, attrs));
+ assertFalse(getVisitReserved(tag, false, false, attrs));
}
-
- private boolean getVisitReserved(String tag, boolean resUrl, boolean resTag) throws Exception {
+
+ private boolean getVisitReserved(String tag, boolean resUrl, boolean resTag, String ... attrs) throws Exception {
// Reserved when lower-case and both URL and Tag reserved.
String attrName = ProxyingVisitor.RESOURCE_TAGS.get(tag.toLowerCase());
- Node node = elem(tag, attrName != null ? attrName : "src", URL_STRING);
+ attrName = attrName != null ? attrName : "src";
+
+ ArrayList <String> attrsList = Lists.newArrayList(attrs);
+ attrsList.add(0, attrName);
+ attrsList.add(1, URL_STRING);
+ attrs = attrsList.toArray(attrs);
+ Node node = elem(tag, attrs);
ContentRewriterFeature.Config config = createMock(ContentRewriterFeature.Config.class);
expect(config.shouldRewriteURL(URL_STRING)).andReturn(resUrl).anyTimes();
expect(config.shouldRewriteTag(tag.toLowerCase())).andReturn(resTag).anyTimes();
- replay(config);
-
+ replay(config);
+
ProxyingVisitor rewriter = new ProxyingVisitor(config, null);
VisitStatus status = rewriter.visit(null, node);
verify(config);
-
+
return status != VisitStatus.BYPASS;
}