[java-opensaml] branch master updated: OSJ-218: Replace deprecated URL encoder in TemplateRequestURLBuilder
Brent Putman
putmanb at georgetown.edu
Wed Sep 12 21:11:34 EDT 2018
This is an automated email from the git hooks/post-receive script.
putmanb pushed a commit to branch master
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=135e15e85a99f2e0c7fddf49e378a344b042d952
The following commit(s) were added to refs/heads/master by this push:
new 135e15e OSJ-218: Replace deprecated URL encoder in TemplateRequestURLBuilder
135e15e is described below
commit 135e15e85a99f2e0c7fddf49e378a344b042d952
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Sep 12 20:14:37 2018 -0400
OSJ-218: Replace deprecated URL encoder in TemplateRequestURLBuilder
---
.../resolver/impl/TemplateRequestURLBuilder.java | 137 +++++++++++++++++----
.../impl/TemplateRequestURLBuilderTest.java | 42 ++++++-
2 files changed, 153 insertions(+), 26 deletions(-)
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilder.java
index 209ee29..f4514ae 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilder.java
@@ -23,21 +23,23 @@ import java.nio.charset.StandardCharsets;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
-import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.logic.Constraint;
-import net.shibboleth.utilities.java.support.net.URISupport;
-import net.shibboleth.utilities.java.support.primitive.StringSupport;
-import net.shibboleth.utilities.java.support.velocity.Template;
-
import org.apache.velocity.VelocityContext;
import org.apache.velocity.app.VelocityEngine;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Function;
+import com.google.common.net.UrlEscapers;
+
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
+import net.shibboleth.utilities.java.support.velocity.Template;
/**
- * Function which produces a URL by substituting a an entity ID value into a Velocity template string.
+ * Function which produces a URL by substituting an entity ID value into a Velocity template string.
*
* <p>
* The entity ID will be replaced in the template string according to the template variable <code>entityID</code>,
@@ -45,12 +47,26 @@ import com.google.common.base.Function;
* </p>
*
* <p>
- * If the value of the <code>encoded</code> parameter is <code>true</code> then the entity ID will be URL encoded prior
- * to substitution. Otherwise, the literal value of the entity ID will be substituted.
+ * The value of the <code>encodingStyle</code> parameter determines whether and how the entity ID will be encoded prior
+ * to substitution, and accepts an enum value from {@link EncodingStyle}.
+ * Legacy deprecated constructors accept an <code>encoded</code> parameter, where <code>true</code>
+ * means {@link EncodingStyle#form} and <code>false</code> means {@link EncodingStyle#none}.
* </p>
*
*/
public class TemplateRequestURLBuilder implements Function<String, String> {
+
+ /** EntityID Encoding style. */
+ public enum EncodingStyle {
+ /** No encoding. */
+ none,
+ /** URL form encoding. @see {@link UrlEscapers#urlFormParameterEscaper()} */
+ form,
+ /** URL path encoding. @see {@link UrlEscapers#urlPathSegmentEscaper()} */
+ path,
+ /** URL fragment encoding. @see {@link UrlEscapers#urlFragmentEscaper()} */
+ fragment
+ };
/** The Velocity context variable name for the entity ID. */
public static final String CONTEXT_KEY_ENTITY_ID = "entityID";
@@ -67,8 +83,8 @@ public class TemplateRequestURLBuilder implements Function<String, String> {
/** Function which transforms the entityID prior to substitution into the template. */
private Function<String, String> transformer;
- /** Flag indicating whether to URL-encode the entity ID value before substitution. */
- private boolean encodeEntityID;
+ /** Enum value indicating whether and how to encode the entity ID value before substitution. */
+ private EncodingStyle entityIDEncodingStyle;
/**
* Constructor.
@@ -77,11 +93,18 @@ public class TemplateRequestURLBuilder implements Function<String, String> {
*
* @param engine the {@link VelocityEngine} instance to use
* @param templateString the Velocity template string
- * @param encoded true if entity ID should be URL-encoded prior to substitution, false otherwise
+ * @param encoded true if entity ID should be URL form-encoded prior to substitution, false otherwise
+ *
+ * @deprecated Replacement is the variant which accepts an instance of {@link EncodingStyle}
*/
+ @Deprecated
public TemplateRequestURLBuilder(@Nonnull final VelocityEngine engine,
@Nonnull @NotEmpty final String templateString, final boolean encoded) {
- this(engine, templateString, encoded, null, StandardCharsets.US_ASCII);
+ this(engine, templateString, encoded ? EncodingStyle.form : EncodingStyle.none, null,
+ StandardCharsets.US_ASCII);
+
+ DeprecationSupport.warnOnce(ObjectType.METHOD, getClass().getName() + ".constructor", null,
+ "variant accepting EncodingStyle enum");
}
/**
* Constructor.
@@ -91,12 +114,19 @@ public class TemplateRequestURLBuilder implements Function<String, String> {
* @param engine the {@link VelocityEngine} instance to use
* @param templateString the Velocity template string
* @param transform function which transforms the entityID prior to substitution, may be null
- * @param encoded true if entity ID should be URL-encoded prior to substitution, false otherwise
+ * @param encoded true if entity ID should be URL form-encoded prior to substitution, false otherwise
+ *
+ * @deprecated Replacement is the variant which accepts an instance of {@link EncodingStyle}
*/
+ @Deprecated
public TemplateRequestURLBuilder(@Nonnull final VelocityEngine engine,
@Nonnull @NotEmpty final String templateString, final boolean encoded,
@Nullable final Function<String, String> transform) {
- this(engine, templateString, encoded, transform, StandardCharsets.US_ASCII);
+ this(engine, templateString, encoded ? EncodingStyle.form : EncodingStyle.none, transform,
+ StandardCharsets.US_ASCII);
+
+ DeprecationSupport.warnOnce(ObjectType.METHOD, getClass().getName() + ".constructor", null,
+ "variant accepting EncodingStyle enum");
}
/**
@@ -104,13 +134,66 @@ public class TemplateRequestURLBuilder implements Function<String, String> {
*
* @param engine the {@link VelocityEngine} instance to use
* @param templateString the Velocity template string
- * @param encoded true if entity ID should be URL-encoded prior to substitution, false otherwise
+ * @param encoded true if entity ID should be URL form-encoded prior to substitution, false otherwise
* @param transform function which transforms the entityID prior to substitution, may be null
* @param charSet character set of the template, may be null
+ *
+ * @deprecated Replacement is the variant which accepts an instance of {@link EncodingStyle}
*/
+ @Deprecated
public TemplateRequestURLBuilder(@Nonnull final VelocityEngine engine,
@Nonnull @NotEmpty final String templateString, final boolean encoded,
@Nullable final Function<String, String> transform, @Nullable final Charset charSet) {
+ this(engine, templateString, encoded ? EncodingStyle.form : EncodingStyle.none, transform, charSet);
+
+ DeprecationSupport.warnOnce(ObjectType.METHOD, getClass().getName() + ".constructor", null,
+ "variant accepting EncodingStyle enum");
+ }
+
+ /**
+ * Constructor.
+ *
+ * <p>The template character set will be US ASCII.</p>
+ *
+ * @param engine the {@link VelocityEngine} instance to use
+ * @param templateString the Velocity template string
+ * @param encodingStyle the style for encoding the entity ID prior to substitution,
+ * null means {@link EncodingStyle#none}
+ */
+ public TemplateRequestURLBuilder(@Nonnull final VelocityEngine engine,
+ @Nonnull @NotEmpty final String templateString, @Nullable final EncodingStyle encodingStyle) {
+ this(engine, templateString, encodingStyle, null, StandardCharsets.US_ASCII);
+ }
+ /**
+ * Constructor.
+ *
+ * <p>The template character set will be US ASCII.</p>
+ *
+ * @param engine the {@link VelocityEngine} instance to use
+ * @param templateString the Velocity template string
+ * @param transform function which transforms the entityID prior to substitution, may be null
+ * @param encodingStyle the style for encoding the entity ID prior to substitution,
+ * null means {@link EncodingStyle#none}
+ */
+ public TemplateRequestURLBuilder(@Nonnull final VelocityEngine engine,
+ @Nonnull @NotEmpty final String templateString, @Nullable final EncodingStyle encodingStyle,
+ @Nullable final Function<String, String> transform) {
+ this(engine, templateString, encodingStyle, transform, StandardCharsets.US_ASCII);
+ }
+
+ /**
+ * Constructor.
+ *
+ * @param engine the {@link VelocityEngine} instance to use
+ * @param templateString the Velocity template string
+ * @param encodingStyle the style for encoding the entity ID prior to substitution,
+ * null means {@link EncodingStyle#none}
+ * @param transform function which transforms the entityID prior to substitution, may be null
+ * @param charSet character set of the template, may be null
+ */
+ public TemplateRequestURLBuilder(@Nonnull final VelocityEngine engine,
+ @Nonnull @NotEmpty final String templateString, final EncodingStyle encodingStyle,
+ @Nullable final Function<String, String> transform, @Nullable final Charset charSet) {
Constraint.isNotNull(engine, "VelocityEngine was null");
@@ -125,7 +208,7 @@ public class TemplateRequestURLBuilder implements Function<String, String> {
template = Template.fromTemplate(engine, trimmedTemplate);
}
- encodeEntityID = encoded;
+ entityIDEncodingStyle = encodingStyle != null ? encodingStyle : EncodingStyle.none;
}
/** {@inheritDoc} */
@@ -145,10 +228,22 @@ public class TemplateRequestURLBuilder implements Function<String, String> {
}
final VelocityContext context = new VelocityContext();
- if (encodeEntityID) {
- context.put(CONTEXT_KEY_ENTITY_ID, URISupport.doURLEncode(entityID));
- } else {
- context.put(CONTEXT_KEY_ENTITY_ID, entityID);
+ switch (entityIDEncodingStyle) {
+ case none:
+ context.put(CONTEXT_KEY_ENTITY_ID, entityID);
+ break;
+ case form:
+ context.put(CONTEXT_KEY_ENTITY_ID, UrlEscapers.urlFormParameterEscaper().escape(entityID));
+ break;
+ case path:
+ context.put(CONTEXT_KEY_ENTITY_ID, UrlEscapers.urlPathSegmentEscaper().escape(entityID));
+ break;
+ case fragment:
+ context.put(CONTEXT_KEY_ENTITY_ID, UrlEscapers.urlFragmentEscaper().escape(entityID));
+ break;
+ default:
+ log.warn("An unsupported EncodingStyle value was seen, treating as 'none': {}", entityIDEncodingStyle);
+ context.put(CONTEXT_KEY_ENTITY_ID, entityID);
}
try {
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilderTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilderTest.java
index d9b7b8d..fc6e4aa 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilderTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/TemplateRequestURLBuilderTest.java
@@ -22,6 +22,7 @@ import javax.annotation.Nullable;
import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
import org.apache.velocity.app.VelocityEngine;
+import org.opensaml.saml.metadata.resolver.impl.TemplateRequestURLBuilder.EncodingStyle;
import org.testng.Assert;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
@@ -41,27 +42,58 @@ public class TemplateRequestURLBuilderTest {
}
@Test
- public void testEncodedQueryParam() {
+ @SuppressWarnings("deprecation")
+ public void testEncodedQueryParamLegacy() {
function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/?entity=${entityID}", true);
Assert.assertEquals(function.apply("http://example.org/idp"), "http://metadata.example.org/?entity=http%3A%2F%2Fexample.org%2Fidp");
}
@Test
- public void testMDQStyle() {
+ @SuppressWarnings("deprecation")
+ public void testMDQStyleLegacy() {
function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/entities/${entityID}", true);
Assert.assertEquals(function.apply("http://example.org/idp"), "http://metadata.example.org/entities/http%3A%2F%2Fexample.org%2Fidp");
}
@Test
- public void testWellKnownLocationStyle() {
+ @SuppressWarnings("deprecation")
+ public void testWellKnownLocationStyleLegacy() {
function = new TemplateRequestURLBuilder(engine, "${entityID}", false);
Assert.assertEquals(function.apply("http://example.org/idp"), "http://example.org/idp");
}
@Test
+ public void testEncodedQueryParam() {
+ function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/?entity=${entityID}", EncodingStyle.form);
+
+ Assert.assertEquals(function.apply("http://example.org/idp"), "http://metadata.example.org/?entity=http%3A%2F%2Fexample.org%2Fidp");
+ }
+
+ @Test
+ public void testEncodedPath() {
+ function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/entities/${entityID}", EncodingStyle.path);
+
+ Assert.assertEquals(function.apply("http://example.org/idp"), "http://metadata.example.org/entities/http:%2F%2Fexample.org%2Fidp");
+ }
+
+ @Test
+ public void testEncodedFragment() {
+ function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/entities#${entityID}", EncodingStyle.fragment);
+
+ Assert.assertEquals(function.apply("http://example.org/idp"), "http://metadata.example.org/entities#http://example.org/idp");
+ }
+
+ @Test
+ public void testWellKnownLocationStyle() {
+ function = new TemplateRequestURLBuilder(engine, "${entityID}", EncodingStyle.none);
+
+ Assert.assertEquals(function.apply("http://example.org/idp"), "http://example.org/idp");
+ }
+
+ @Test
public void testTransformer() {
Function<String,String> transformer = new Function<String, String>() {
@Nullable public String apply(@Nullable String input) {
@@ -69,14 +101,14 @@ public class TemplateRequestURLBuilderTest {
}
};
- function = new TemplateRequestURLBuilder(engine, "${entityID}", false, transformer);
+ function = new TemplateRequestURLBuilder(engine, "${entityID}", EncodingStyle.none, transformer);
Assert.assertEquals(function.apply("http://example.org/idp"), "HTTP://EXAMPLE.ORG/IDP");
}
@Test(expectedExceptions=ConstraintViolationException.class)
public void testNullEntityID() {
- function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/?entity=${entityID}", true);
+ function = new TemplateRequestURLBuilder(engine, "http://metadata.example.org/?entity=${entityID}", EncodingStyle.form);
function.apply(null);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list