Hi Deepak,

Thanks for the interest. I had exactly the same idea initially. There are indeed 600+ 
"'html" cases vs 80+ "url" ones.

But then I thought that some people will maybe not notice the difference and will always use the shorten version (the "html" one). Because it's about security (low one fortunately) I decided to not go that way.

This said, I'm not against it, simply it should be discussed ;)

Also I did not backport (yet) to older releases than R14, but this should be considered indeed. You can never be sure someone is not able to inject some SQL somehow to create a static XSS exploit :/

Cheers

Jacques



Le 12/10/2015 09:25, Deepak Dixit a écrit :
Hi Jacques,

I think we fixed all these ContentWrapper* code in trunk code base,  but it
will affect lot of custom code as well.
So IMO  we have to maintain backward compatibility as well.

Can we add an get method that will use the default encoding as html?

As we generally used ContentWrapper to render the Content (generally
html/text) so we can add get method with default encoding as html e.g.

{code}
public String get(String contentTypeId) {
  return get (contentTypeId, "html")
}
{code}

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com

On Sat, Oct 10, 2015 at 4:08 PM, <jler...@apache.org
<javascript:_e(%7B%7D,'cvml','jler...@apache.org');>> wrote:

Author: jleroux
Date: Sat Oct 10 10:38:49 2015
New Revision: 1707857

URL: http://svn.apache.org/viewvc?rev=1707857&view=rev
Log:
Fixes "ecommerce screen exception"
https://issues.apache.org/jira/browse/OFBIZ-6665 reported by Wai

I missed this hidden ProductConfigItemContentWrapper class while working
on r1705329 and sequels. I checked there are no other *ContentWrapper
classes left now.

Modified:

ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java

ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWrapper.java

ofbiz/trunk/specialpurpose/ecommerce/webapp/ecommerce/catalog/configproductdetail.ftl

Modified:
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java?rev=1707857&r1=1707856&r2=1707857&view=diff

==============================================================================
---
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
(original)
+++
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
Sat Oct 10 10:38:49 2015
@@ -22,7 +22,6 @@ import java.io.IOException;
  import java.io.StringWriter;
  import java.io.Writer;
  import java.util.HashMap;
-import java.util.List;
  import java.util.Locale;
  import java.util.Map;

@@ -30,9 +29,13 @@ import javax.servlet.http.HttpServletReq

  import org.ofbiz.base.util.Debug;
  import org.ofbiz.base.util.GeneralException;
+import org.ofbiz.base.util.StringUtil;
+import org.ofbiz.base.util.StringUtil.StringWrapper;
+import org.ofbiz.base.util.UtilCodec;
  import org.ofbiz.base.util.UtilHttp;
  import org.ofbiz.base.util.UtilValidate;
  import org.ofbiz.content.content.ContentWorker;
+import org.ofbiz.content.content.ContentWrapper;
  import org.ofbiz.entity.Delegator;
  import org.ofbiz.entity.DelegatorFactory;
  import org.ofbiz.entity.GenericValue;
@@ -46,7 +49,7 @@ import org.ofbiz.service.ServiceContaine
   * Product Config Item Content Worker: gets product content to display
   */
  @SuppressWarnings("serial")
-public class ProductConfigItemContentWrapper implements
java.io.Serializable {
+public class ProductConfigItemContentWrapper implements ContentWrapper {

      public static final String module =
ProductConfigItemContentWrapper.class.getName();

@@ -83,8 +86,8 @@ public class ProductConfigItemContentWra
          this.mimeTypeId = "text/html";
      }

-    public String get(String confItemContentTypeId) {
-        return getProductConfigItemContentAsText(productConfigItem,
confItemContentTypeId, locale, mimeTypeId, getDelegator(), getDispatcher());
+    public StringWrapper get(String confItemContentTypeId, String
encoderType) {
+        return
StringUtil.makeStringWrapper(getProductConfigItemContentAsText(productConfigItem,
confItemContentTypeId, locale, mimeTypeId, getDelegator(), getDispatcher(),
encoderType));
      }

      public Delegator getDelegator() {
@@ -101,32 +104,36 @@ public class ProductConfigItemContentWra
          return dispatcher;
      }

-    public static String getProductConfigItemContentAsText(GenericValue
productConfigItem, String confItemContentTypeId, HttpServletRequest
request) {
+    public static String getProductConfigItemContentAsText(GenericValue
productConfigItem, String confItemContentTypeId, HttpServletRequest
request, String encoderType) {
          LocalDispatcher dispatcher = (LocalDispatcher)
request.getAttribute("dispatcher");
-        return getProductConfigItemContentAsText(productConfigItem,
confItemContentTypeId, UtilHttp.getLocale(request), "text/html",
productConfigItem.getDelegator(), dispatcher);
+        return getProductConfigItemContentAsText(productConfigItem,
confItemContentTypeId, UtilHttp.getLocale(request), "text/html",
productConfigItem.getDelegator(), dispatcher, encoderType);
      }

-    public static String getProductConfigItemContentAsText(GenericValue
productConfigItem, String confItemContentTypeId, Locale locale,
LocalDispatcher dispatcher) {
-        return getProductConfigItemContentAsText(productConfigItem,
confItemContentTypeId, locale, null, null, dispatcher);
+    public static String getProductConfigItemContentAsText(GenericValue
productConfigItem, String confItemContentTypeId, Locale locale,
LocalDispatcher dispatcher, String encoderType) {
+        return getProductConfigItemContentAsText(productConfigItem,
confItemContentTypeId, locale, null, null, dispatcher, encoderType);
      }

-    public static String getProductConfigItemContentAsText(GenericValue
productConfigItem, String confItemContentTypeId, Locale locale, String
mimeTypeId, Delegator delegator, LocalDispatcher dispatcher) {
+    public static String getProductConfigItemContentAsText(GenericValue
productConfigItem, String confItemContentTypeId, Locale locale, String
mimeTypeId, Delegator delegator, LocalDispatcher dispatcher, String
encoderType) {
+        UtilCodec.SimpleEncoder encoder =
UtilCodec.getEncoder(encoderType);
          String candidateFieldName =
ModelUtil.dbNameToVarName(confItemContentTypeId);
          try {
              Writer outWriter = new StringWriter();
              getProductConfigItemContentAsText(null, productConfigItem,
confItemContentTypeId, locale, mimeTypeId, delegator, dispatcher,
outWriter);
              String outString = outWriter.toString();
              if (outString.length() > 0) {
-                return outString;
+                return encoder.encode(outString);
              } else {
-                return null;
+                String candidateOut =
productConfigItem.getModelEntity().isField(candidateFieldName) ?
productConfigItem.getString(candidateFieldName): "";
+                return candidateOut == null? "" :
encoder.encode(candidateOut);
              }
          } catch (GeneralException e) {
              Debug.logError(e, "Error rendering ProdConfItemContent,
inserting empty String", module);
-            return productConfigItem.getString(candidateFieldName);
+            String candidateOut =
productConfigItem.getModelEntity().isField(candidateFieldName) ?
productConfigItem.getString(candidateFieldName): "";
+            return candidateOut == null? "" :
encoder.encode(candidateOut);
          } catch (IOException e) {
              Debug.logError(e, "Error rendering ProdConfItemContent,
inserting empty String", module);
-            return productConfigItem.getString(candidateFieldName);
+            String candidateOut =
productConfigItem.getModelEntity().isField(candidateFieldName) ?
productConfigItem.getString(candidateFieldName): "";
+            return candidateOut == null? "" :
encoder.encode(candidateOut);
          }
      }


Modified:
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWrapper.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWrapper.java?rev=1707857&r1=1707856&r2=1707857&view=diff

==============================================================================
---
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWrapper.java
(original)
+++
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWrapper.java
Sat Oct 10 10:38:49 2015
@@ -458,7 +458,7 @@ public class ProductConfigWrapper implem
                  question = configItemAssoc.getString("description");
              } else {
                  if (content != null) {
-                    question = content.get("DESCRIPTION");
+                    question = content.get("DESCRIPTION",
"html").toString();
                  } else {
                      question = (configItem.getString("description") !=
null? configItem.getString("description"): "");
                  }
@@ -472,7 +472,7 @@ public class ProductConfigWrapper implem
                  description =
configItemAssoc.getString("longDescription");
              } else {
                  if (content != null) {
-                    description = content.get("LONG_DESCRIPTION");
+                    description = content.get("LONG_DESCRIPTION",
"html").toString();
                  } else {
                      description =
(configItem.getString("longDescription") != null?
configItem.getString("longDescription"): "");
                  }
@@ -665,7 +665,7 @@ public class ProductConfigWrapper implem
          }

          public String getOptionName(Locale locale) {
-
+
              return (configOption.getString("configOptionName") != null?
(String) configOption.get("configOptionName", locale): "no option name");
          }


Modified:
ofbiz/trunk/specialpurpose/ecommerce/webapp/ecommerce/catalog/configproductdetail.ftl
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/ecommerce/webapp/ecommerce/catalog/configproductdetail.ftl?rev=1707857&r1=1707856&r2=1707857&view=diff

==============================================================================
---
ofbiz/trunk/specialpurpose/ecommerce/webapp/ecommerce/catalog/configproductdetail.ftl
(original)
+++
ofbiz/trunk/specialpurpose/ecommerce/webapp/ecommerce/catalog/configproductdetail.ftl
Sat Oct 10 10:38:49 2015
@@ -456,8 +456,8 @@ function getConfigDetails(event) {
                <div>${question.question}</div>
                <#if question.isFirst()>
                  <a
name='#${question.getConfigItem().getString("configItemId")}'></a>
-                <div>${question.description!}</div>
-                <#assign instructions =
question.content.get("INSTRUCTIONS")!>
+                <div>${StringUtil.wrapString(question.description!)}</div>
+                <#assign instructions =
question.content.get("INSTRUCTIONS", "html")!>
                  <#if instructions?has_content>
                    <a
href="javascript:showErrorAlert("${uiLabelMap.CommonErrorMessage2}","${instructions}");"
class="buttontext">Instructions</a>
                  </#if>




Reply via email to