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, <[hidden email] <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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> > > > -- Thanks & Regards -- Deepak Dixit www.hotwaxsystems.com |
Administrator
|
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, <[hidden email] > <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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> >> >> >> > |
Hi Jacques, Deepak,
we could add the method that Deepak is proposing, for backward compatibility, with a deprecation warning and plan for removing it before we create our next release branch. Jacopo On Mon, Oct 12, 2015 at 11:50 PM, Jacques Le Roux < [hidden email]> wrote: > 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, <[hidden email] >> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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> >>> >>> >>> >>> >> |
Administrator
|
Yes that would work for me, other opinions?
Jacques Le 13/10/2015 03:31, Jacopo Cappellato a écrit : > Hi Jacques, Deepak, > > we could add the method that Deepak is proposing, for backward > compatibility, with a deprecation warning and plan for removing it before > we create our next release branch. > > Jacopo > > On Mon, Oct 12, 2015 at 11:50 PM, Jacques Le Roux < > [hidden email]> wrote: > >> 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, <[hidden email] >>> <javascript:_e(%7B%7D,'cvml','[hidden email]');>> 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> >>>> >>>> >>>> >>>> |
Free forum by Nabble | Edit this page |