Author: asanso Date: Wed Jul 29 10:09:42 2015 New Revision: 1693233 URL: http://svn.apache.org/r1693233 Log: SLING-4883 - Extend content disposition filter protection to jcr:data
Modified: sling/trunk/contrib/extensions/security/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java sling/trunk/contrib/extensions/security/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java Modified: sling/trunk/contrib/extensions/security/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java URL: http://svn.apache.org/viewvc/sling/trunk/contrib/extensions/security/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java?rev=1693233&r1=1693232&r2=1693233&view=diff ============================================================================== --- sling/trunk/contrib/extensions/security/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java (original) +++ sling/trunk/contrib/extensions/security/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java Wed Jul 29 10:09:42 2015 @@ -67,6 +67,13 @@ public class ContentDispositionFilter im "Invalid entries are logged and ignored." , unbounded = PropertyUnbounded.ARRAY, value = { "" }) private static final String PROP_CONTENT_DISPOSTION_PATHS = "sling.content.disposition.paths"; + + private static final boolean DEFAULT_ENABLE_CONTENT_DISPOSTION_ALL_PATHS = false; + @Property(boolValue = DEFAULT_ENABLE_CONTENT_DISPOSTION_ALL_PATHS , + label = "Enable Content Disposition for all paths", + description ="This flag controls whether to enable" + + " Content Disposition for all paths.") + private static final String PROP_ENABLE_CONTENT_DISPOSTION_ALL_PATHS = "sling.content.disposition.all.paths"; /** * Set of paths @@ -80,6 +87,8 @@ public class ContentDispositionFilter im private Map<String, Set<String>> contentTypesMapping; + private boolean enableContentDispositionAllPaths; + @Activate private void activate(final ComponentContext ctx) { final Dictionary props = ctx.getProperties(); @@ -131,8 +140,10 @@ public class ContentDispositionFilter im contentDispositionPathsPfx = pfxs.toArray(new String[pfxs.size()]); contentTypesMapping = contentTypesMap.isEmpty()?Collections.<String, Set<String>>emptyMap(): contentTypesMap; - logger.info("Initialized. content disposition paths: {}, content disposition paths-pfx {}", new Object[]{ - contentDispositionPaths, contentDispositionPathsPfx} + enableContentDispositionAllPaths = PropertiesUtil.toBoolean(props.get(PROP_ENABLE_CONTENT_DISPOSTION_ALL_PATHS),DEFAULT_ENABLE_CONTENT_DISPOSTION_ALL_PATHS); + + logger.info("Initialized. content disposition paths: {}, content disposition paths-pfx {}. Enable Content Disposition for all paths is set to {}", new Object[]{ + contentDispositionPaths, contentDispositionPathsPfx, enableContentDispositionAllPaths} ); } @@ -203,33 +214,40 @@ public class ContentDispositionFilter im } request.setAttribute(ATTRIBUTE_NAME, type); Resource resource = request.getResource(); - String resourcePath = resource.getPath(); - if (contentDispositionPaths.contains(resourcePath)) { + if (enableContentDispositionAllPaths) { + setContentDisposition(resource); + } else { + String resourcePath = resource.getPath(); - if (contentTypesMapping.containsKey(resourcePath)) { - Set <String> exceptions = contentTypesMapping.get(resourcePath); - if (!exceptions.contains(type)) { - setContentDisposition(resource); - } - } else { - setContentDisposition(resource); - } - } - - for (String path : contentDispositionPathsPfx) { - if (resourcePath.startsWith(path)) { - if (contentTypesMapping.containsKey(path)) { - Set <String> exceptions = contentTypesMapping.get(path); + boolean contentDispositionAdded = false; + if (contentDispositionPaths.contains(resourcePath)) { + + if (contentTypesMapping.containsKey(resourcePath)) { + Set <String> exceptions = contentTypesMapping.get(resourcePath); if (!exceptions.contains(type)) { - setContentDisposition(resource); - break; + contentDispositionAdded = setContentDisposition(resource); } } else { - setContentDisposition(resource); - break; + contentDispositionAdded = setContentDisposition(resource); } + } + if (!contentDispositionAdded) { + for (String path : contentDispositionPathsPfx) { + if (resourcePath.startsWith(path)) { + if (contentTypesMapping.containsKey(path)) { + Set <String> exceptions = contentTypesMapping.get(path); + if (!exceptions.contains(type)) { + setContentDisposition(resource); + break; + } + } else { + setContentDisposition(resource); + break; + } + } + } } } super.setContentType(type); @@ -237,10 +255,13 @@ public class ContentDispositionFilter im //---------- PRIVATE METHODS --------- - private void setContentDisposition(Resource resource) { + private boolean setContentDisposition(Resource resource) { + boolean contentDispositionAdded = false; if (!this.containsHeader(CONTENT_DISPOSTION) && this.isJcrData(resource)) { this.addHeader(CONTENT_DISPOSTION, CONTENT_DISPOSTION_ATTACHMENT); + contentDispositionAdded = true; } + return contentDispositionAdded; } private boolean isJcrData(Resource resource){ Modified: sling/trunk/contrib/extensions/security/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java URL: http://svn.apache.org/viewvc/sling/trunk/contrib/extensions/security/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java?rev=1693233&r1=1693232&r2=1693233&view=diff ============================================================================== --- sling/trunk/contrib/extensions/security/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java (original) +++ sling/trunk/contrib/extensions/security/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java Wed Jul 29 10:09:42 2015 @@ -869,7 +869,8 @@ public class ContentDispositionFilterTes rewriterResponse.setContentType("text/html"); rewriterResponse.setContentType("text/html"); Assert.assertEquals(1, counter.intValue()); - } + } + /** * Test repeated setContentType calls don't add multiple headers, case 2 changing mime type * @throws Throwable @@ -933,6 +934,126 @@ public class ContentDispositionFilterTes Assert.assertEquals(1, counter.intValue()); } + + @Test + public void test_doFilter17() throws Throwable{ + final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class); + final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class); + final Resource resource = context.mock(Resource.class, "resource" ); + final ValueMap properties = context.mock(ValueMap.class); + contentDispositionFilter = new ContentDispositionFilter(); + + final ComponentContext ctx = context.mock(ComponentContext.class); + final Dictionary props = new Hashtable<String, String[]>(); + props.put("sling.content.disposition.paths", new String []{"/content/usergenerated"}); + props.put("sling.content.disposition.all.paths", false); + + context.checking(new Expectations() { + { + allowing(ctx).getProperties(); + will(returnValue(props)); + + } + }); + PrivateAccessor.invoke(contentDispositionFilter,"activate", new Class[]{ComponentContext.class},new Object[]{ctx}); + final AtomicInteger counter = new AtomicInteger(); + final ContentDispositionFilter.RewriterResponse rewriterResponse = contentDispositionFilter. new RewriterResponse(request, response) { + public void addHeader(String name, String value) { + counter.incrementAndGet(); + } + }; + + + context.checking(new Expectations() { + { + exactly(1).of(response).containsHeader("Content-Disposition"); + will(returnValue(false)); + exactly(1).of(response).containsHeader("Content-Disposition"); + will(returnValue(true)); + exactly(1).of(request).getAttribute(RewriterResponse.ATTRIBUTE_NAME); + will(returnValue(null)); + exactly(1).of(request).getAttribute(RewriterResponse.ATTRIBUTE_NAME); + will(returnValue("text/html")); + allowing(request).setAttribute(RewriterResponse.ATTRIBUTE_NAME, "text/xml"); + allowing(request).setAttribute(RewriterResponse.ATTRIBUTE_NAME, "text/html"); + allowing(request).getResource(); + will(returnValue(resource)); + allowing(resource).getPath(); + will(returnValue("/content/other")); + allowing(resource).adaptTo(ValueMap.class); + will(returnValue(properties)); + allowing(properties).containsKey(PROP_JCR_DATA); + will(returnValue(true)); + allowing(response).setContentType("text/html"); + allowing(response).setContentType("text/xml"); + //CONTENT DISPOSITION IS NOT SET + never(response).addHeader("Content-Disposition", "attachment"); + } + }); + rewriterResponse.setContentType("text/html"); + Assert.assertEquals(0, counter.intValue()); + } + + + @Test + public void test_doFilter18() throws Throwable{ + final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class); + final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class); + final Resource resource = context.mock(Resource.class, "resource" ); + final ValueMap properties = context.mock(ValueMap.class); + contentDispositionFilter = new ContentDispositionFilter(); + + final ComponentContext ctx = context.mock(ComponentContext.class); + final Dictionary props = new Hashtable<String, String[]>(); + props.put("sling.content.disposition.paths", new String []{"/content/usergenerated"}); + props.put("sling.content.disposition.all.paths", true); + + context.checking(new Expectations() { + { + allowing(ctx).getProperties(); + will(returnValue(props)); + + } + }); + PrivateAccessor.invoke(contentDispositionFilter,"activate", new Class[]{ComponentContext.class},new Object[]{ctx}); + final AtomicInteger counter = new AtomicInteger(); + final ContentDispositionFilter.RewriterResponse rewriterResponse = contentDispositionFilter. new RewriterResponse(request, response) { + public void addHeader(String name, String value) { + counter.incrementAndGet(); + } + }; + + + context.checking(new Expectations() { + { + exactly(1).of(response).containsHeader("Content-Disposition"); + will(returnValue(false)); + exactly(1).of(response).containsHeader("Content-Disposition"); + will(returnValue(true)); + exactly(1).of(request).getAttribute(RewriterResponse.ATTRIBUTE_NAME); + will(returnValue(null)); + exactly(1).of(request).getAttribute(RewriterResponse.ATTRIBUTE_NAME); + will(returnValue("text/html")); + allowing(request).setAttribute(RewriterResponse.ATTRIBUTE_NAME, "text/xml"); + allowing(request).setAttribute(RewriterResponse.ATTRIBUTE_NAME, "text/html"); + allowing(request).getResource(); + will(returnValue(resource)); + allowing(resource).getPath(); + will(returnValue("/content/other")); + allowing(resource).adaptTo(ValueMap.class); + will(returnValue(properties)); + allowing(properties).containsKey(PROP_JCR_DATA); + will(returnValue(true)); + allowing(response).setContentType("text/html"); + allowing(response).setContentType("text/xml"); + //CONTENT DISPOSITION IS SET + exactly(1).of(response).addHeader("Content-Disposition", "attachment"); + } + }); + rewriterResponse.setContentType("text/html"); + Assert.assertEquals(1, counter.intValue()); + } + @Test public void test_isJcrData1() throws Throwable { contentDispositionFilter = new ContentDispositionFilter();