From cb5b3f190167f9e8450f0e6d74e433fab59790af Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Mon, 22 Mar 2021 03:05:19 +0100 Subject: [PATCH 1/9] TagDataServlet input validation. --- .../webservices/tagdata/TagDataServlet.java | 58 +++++++++++++------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java index 9d2fa31f8c..527731953d 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java @@ -26,6 +26,8 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.text.StringEscapeUtils; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.URLStrategy; import org.apache.roller.weblogger.business.WeblogEntryManager; @@ -44,7 +46,7 @@ * These URLs are supported: * * See the * Tag Data API proposal for details. @@ -70,36 +72,56 @@ public void doGet( HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - String[] pathInfo = new String[0]; - boolean siteWide; - String handle; - String prefix; - String format = "json"; - int page = 0; - // TODO: last modified or ETag support, caching, etc. + String[] pathInfo = new String[0]; + if (request.getPathInfo() != null) { pathInfo = Utilities.stringToStringArray(request.getPathInfo(),"/"); } + + boolean siteWide; + String handle; + if (pathInfo.length == 0) { siteWide = true; // we'll use the front-page weblog to form URLs handle = WebloggerRuntimeConfig.getProperty("site.frontpage.weblog.handle"); - } else if (pathInfo.length == 2 && "weblog".equals(pathInfo[0])) { + } else if (pathInfo.length == 2 && "weblog".equals(pathInfo[0]) && StringUtils.isAlphanumeric(pathInfo[1])) { siteWide = false; handle = pathInfo[1]; } else { response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); return; } - prefix = request.getParameter("prefix"); + + String prefix = request.getParameter("prefix"); + + if(prefix != null && !StringUtils.isAlphanumeric(prefix)) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); + return; + } + + String format = "json"; // default + if (request.getParameter("format") != null) { + format = request.getParameter("format"); + if(!format.equals("json") || !format.equals("xml")) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); + return; + } + } + + int page = 0; + if(request.getParameter("page") != null) { + try { + page = Integer.parseInt(request.getParameter("page")); + } catch (NumberFormatException notIgnored) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); + return; + } } - try { - page = Integer.parseInt(request.getParameter("page")); - } catch (Exception ignored) {} Weblogger roller = WebloggerFactory.getWeblogger(); List tags; @@ -108,6 +130,10 @@ public void doGet( WeblogManager wmgr = roller.getWeblogManager(); WeblogEntryManager emgr = roller.getWeblogEntryManager(); weblog = wmgr.getWeblogByHandle(handle); + if(weblog == null) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Weblog not found"); + return; + } // get tags, if site-wide then don't specify weblog tags = emgr.getTags(siteWide ? null : weblog, null, prefix, page * MAX, MAX + 1); @@ -119,8 +145,8 @@ public void doGet( if ("json".equals(format)) { response.setContentType("application/json; charset=utf-8"); PrintWriter pw = response.getWriter(); - pw.println("{ \"prefix\": \"" + (prefix == null ? "" : prefix) + "\","); - pw.println(" \"weblog\": \"" + (!siteWide ? handle : "") + "\","); + pw.println("{ \"prefix\": \"" + (prefix == null ? "" : StringEscapeUtils.escapeJson(prefix)) + "\","); + pw.println(" \"weblog\": \"" + (!siteWide ? weblog.getHandle() : "") + "\","); pw.println(" \"tagcounts\": [" ); int count = 0; for (Iterator it = tags.iterator(); it.hasNext();) { @@ -177,8 +203,6 @@ public void doGet( } pw.println(""); response.flushBuffer(); - } else { - response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); } } } From 15e9b06698ea6da12a1b58a182380443c7cefe36 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 18 May 2021 03:08:41 +0200 Subject: [PATCH 2/9] OpenSearchServlet input validation. --- .../ui/rendering/servlets/FeedServlet.java | 2 +- .../opensearch/OpenSearchServlet.java | 66 ++++++++++--------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/FeedServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/FeedServlet.java index e9fbda1b40..c05bdfd56d 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/FeedServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/FeedServlet.java @@ -55,7 +55,7 @@ */ public class FeedServlet extends HttpServlet { - private static Log log = LogFactory.getLog(FeedServlet.class); + private static final Log log = LogFactory.getLog(FeedServlet.class); private WeblogFeedCache weblogFeedCache = null; private SiteWideCache siteWideCache = null; diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/opensearch/OpenSearchServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/opensearch/OpenSearchServlet.java index 9d31a97dfd..fe8e7b57a9 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/opensearch/OpenSearchServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/opensearch/OpenSearchServlet.java @@ -23,7 +23,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.text.StringEscapeUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.URLStrategy; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -31,10 +31,11 @@ import org.apache.roller.weblogger.pojos.Weblog; import org.apache.roller.weblogger.util.Utilities; +import static org.apache.commons.text.StringEscapeUtils.escapeXml11; /** * Return OpenSearch descriptor that describes Roller's search facilities. - * For more informaton see the + * For more information see the * OpenSearch proposal. * @author Dave Johnson (davidm.johnson@sun.com) */ @@ -46,18 +47,19 @@ public void doGet( throws ServletException, IOException { String[] pathInfo = new String[0]; - String handle = null; // Will return descriptor for searching specified blog if (request.getPathInfo() != null) { pathInfo = Utilities.stringToStringArray(request.getPathInfo(), "/"); } + String handle; + if (pathInfo.length == 0) { // URL format: [context]/roller-services/opensearch handle = WebloggerRuntimeConfig.getProperty("site.frontpage.weblog.handle"); - } else if (pathInfo.length == 1) { + } else if (pathInfo.length == 1 && StringUtils.isAlphanumeric(pathInfo[0])) { // URL format: [context]/roller-services/opensearch/[weblog-handle] handle = pathInfo[0]; @@ -65,43 +67,44 @@ public void doGet( response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); return; } - - String shortName = null; - String description = null; - String contact = null; - String searchFeed = null; - String searchPage = null; - URLStrategy strat = WebloggerFactory.getWeblogger().getUrlStrategy(); - Weblog weblog = null; + Weblog weblog; + try { weblog = WebloggerFactory.getWeblogger().getWeblogManager().getWeblogByHandle(handle); + if (weblog == null) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Weblog not found"); + return; + } } catch (WebloggerException ex) { - throw new ServletException("ERROR: fetching specified weblog"); + throw new ServletException("ERROR: fetching specified weblog", ex); } - searchPage = StringEscapeUtils.escapeXml11( - strat.getWeblogSearchPageURLTemplate(weblog)); - searchFeed = StringEscapeUtils.escapeXml11( - strat.getWeblogSearchFeedURLTemplate(weblog)); - boolean siteWide = WebloggerRuntimeConfig.isSiteWideWeblog(handle); - if (siteWide) { - shortName = "[Search Descriptor] " + StringEscapeUtils.escapeXml11( - WebloggerRuntimeConfig.getProperty("site.shortName")); - description = StringEscapeUtils.escapeXml11( - WebloggerRuntimeConfig.getProperty("site.description")); - contact = StringEscapeUtils.escapeXml11( - WebloggerRuntimeConfig.getProperty("site.adminemail")); - + String shortName; + String description; + String contact; + String searchFeed; + String searchPage; + + URLStrategy strat = WebloggerFactory.getWeblogger().getUrlStrategy(); + searchPage = escapeXml11(strat.getWeblogSearchPageURLTemplate(weblog)); + searchFeed = escapeXml11(strat.getWeblogSearchFeedURLTemplate(weblog)); + + if (WebloggerRuntimeConfig.isSiteWideWeblog(handle)) { + + shortName = "[Search Descriptor] " + escapeXml11(WebloggerRuntimeConfig.getProperty("site.shortName")); + description = escapeXml11(WebloggerRuntimeConfig.getProperty("site.description")); + contact = escapeXml11(WebloggerRuntimeConfig.getProperty("site.adminemail")); + } else { - shortName = StringEscapeUtils.escapeXml11(weblog.getName()); - description = StringEscapeUtils.escapeXml11(weblog.getTagline()); - contact = StringEscapeUtils.escapeXml11(weblog.getEmailAddress()); + shortName = escapeXml11(weblog.getName()); + description = escapeXml11(weblog.getTagline()); + contact = escapeXml11(weblog.getEmailAddress()); } response.setContentType("application/opensearchdescription+xml"); - PrintWriter pw = new PrintWriter(response.getWriter()); + PrintWriter pw = response.getWriter(); pw.println(""); pw.println(""); pw.println(" " + shortName + ""); @@ -112,8 +115,7 @@ public void doGet( pw.println(" "); pw.println(""); - pw.flush(); - pw.close(); + pw.flush(); } } From 6ca8db8dcc2e1e2460d0f13d1cdaaa4cb43bd8dd Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Mon, 22 Mar 2021 04:52:02 +0100 Subject: [PATCH 3/9] weblog handle validation. --- .../business/jpa/JPAWeblogManagerImpl.java | 22 ++++++++++++++++--- .../comments/CommentAuthenticatorUtils.java | 8 ++++--- .../ui/rendering/servlets/CommentServlet.java | 6 ++--- .../ui/rendering/servlets/SearchServlet.java | 19 ++++++++-------- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java b/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java index dc03c7614e..14530f55ea 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java @@ -362,11 +362,12 @@ public Weblog getWeblogByHandle(String handle) throws WebloggerException { * Return weblog specified by handle. */ @Override - public Weblog getWeblogByHandle(String handle, Boolean visible) - throws WebloggerException { + public Weblog getWeblogByHandle(String handle, Boolean visible) throws WebloggerException { - if (handle==null) { + if (handle == null) { throw new WebloggerException("Handle cannot be null"); + } else if (!isAlphanumeric(handle)) { + throw new WebloggerException("Invalid handle: '"+handle+"'"); } // check cache first @@ -704,4 +705,19 @@ public long getWeblogCount() throws WebloggerException { return results.get(0); } + /** + * Returns true if alphanumeric or '_'. + */ + private boolean isAlphanumeric(String str) { + if (str == null) { + return false; + } + for (int i = 0; i < str.length(); i++) { + if (!Character.isLetterOrDigit(str.charAt(i)) && str.charAt(i) != '_') { + return false; + } + } + return true; + } + } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/plugins/comments/CommentAuthenticatorUtils.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/plugins/comments/CommentAuthenticatorUtils.java index bb1ebe0f0b..12a78f600a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/plugins/comments/CommentAuthenticatorUtils.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/plugins/comments/CommentAuthenticatorUtils.java @@ -28,16 +28,18 @@ import java.util.Locale; class CommentAuthenticatorUtils { - private static Log log = LogFactory.getLog(CommentAuthenticatorUtils.class); + private static final Log log = LogFactory.getLog(CommentAuthenticatorUtils.class); public static Locale getLocale(HttpServletRequest request) { String handle = request.getParameter("weblog"); try { Weblog weblog = WebloggerFactory.getWeblogger().getWeblogManager().getWeblogByHandle(handle); - return weblog.getLocaleInstance(); + if(weblog != null) { + return weblog.getLocaleInstance(); + } } catch (WebloggerException e) { log.debug("Failed to determine weblog's locale. fallback to the locale of the request", e); - return request.getLocale(); } + return request.getLocale(); } } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/CommentServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/CommentServlet.java index d8bb6bda1e..da50356bf0 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/CommentServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/CommentServlet.java @@ -73,7 +73,7 @@ */ public class CommentServlet extends HttpServlet { - private static Log log = LogFactory.getLog(CommentServlet.class); + private static final Log log = LogFactory.getLog(CommentServlet.class); private CommentAuthenticator authenticator = null; private CommentValidationManager commentValidationManager = null; @@ -202,9 +202,7 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) try { commentRequest = new WeblogCommentRequest(request); - // lookup weblog specified by comment request - weblog = WebloggerFactory.getWeblogger().getWeblogManager() - .getWeblogByHandle(commentRequest.getWeblogHandle()); + weblog = commentRequest.getWeblog(); if (weblog == null) { throw new WebloggerException("unable to lookup weblog: " diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/SearchServlet.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/SearchServlet.java index 79c441f585..729d3bb28a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/SearchServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/servlets/SearchServlet.java @@ -59,7 +59,7 @@ public class SearchServlet extends HttpServlet { private static final long serialVersionUID = 6246730804167411636L; - private static Log log = LogFactory.getLog(SearchServlet.class); + private static final Log log = LogFactory.getLog(SearchServlet.class); // Development theme reloading Boolean themeReload = false; @@ -87,20 +87,19 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) log.debug("Entering"); - Weblog weblog = null; - WeblogSearchRequest searchRequest = null; + Weblog weblog; + WeblogSearchRequest searchRequest; // first off lets parse the incoming request and validate it try { searchRequest = new WeblogSearchRequest(request); // now make sure the specified weblog really exists - weblog = WebloggerFactory - .getWeblogger() - .getWeblogManager() - .getWeblogByHandle(searchRequest.getWeblogHandle(), - Boolean.TRUE); - + weblog = searchRequest.getWeblog(); + if (weblog == null) { + response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Weblog not found"); + return; + } } catch (Exception e) { // invalid search request format or weblog doesn't exist log.debug("error creating weblog search request", e); @@ -229,7 +228,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) } // lookup Renderer we are going to use - Renderer renderer = null; + Renderer renderer; try { log.debug("Looking up renderer"); renderer = RendererManager.getRenderer(page, deviceType); From f02be4cd848853b18fa5db973e617a98142b0a45 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 18 May 2021 03:04:56 +0200 Subject: [PATCH 4/9] WeblogRequest and WeblogFeedRequest input validation. --- .../weblogger/ui/rendering/util/WeblogFeedRequest.java | 7 +++++-- .../weblogger/ui/rendering/util/WeblogRequest.java | 10 +++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogFeedRequest.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogFeedRequest.java index 7b788a715a..5a108553aa 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogFeedRequest.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogFeedRequest.java @@ -22,6 +22,7 @@ import javax.servlet.http.HttpServletRequest; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.WebloggerException; @@ -43,7 +44,7 @@ */ public class WeblogFeedRequest extends WeblogRequest { - private static Log log = LogFactory.getLog(WeblogFeedRequest.class); + private static final Log log = LogFactory.getLog(WeblogFeedRequest.class); private static final String FEED_SERVLET = "/roller-ui/rendering/feed"; @@ -97,7 +98,9 @@ public WeblogFeedRequest(HttpServletRequest request) if(pathInfo != null && pathInfo.trim().length() > 1) { String[] pathElements = pathInfo.split("/"); - if(pathElements.length == 2) { + if(pathElements.length == 2 + && StringUtils.isAlphanumeric(pathElements[0]) + && StringUtils.isAlphanumeric(pathElements[1])) { this.type = pathElements[0]; this.format = pathElements[1]; } else { diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogRequest.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogRequest.java index 2e8468cb9d..37fa6d8067 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogRequest.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/WeblogRequest.java @@ -20,6 +20,7 @@ import java.util.Locale; import javax.servlet.http.HttpServletRequest; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.WebloggerException; @@ -48,7 +49,7 @@ */ public class WeblogRequest extends ParsedRequest { - private static Log log = LogFactory.getLog(WeblogRequest.class); + private static final Log log = LogFactory.getLog(WeblogRequest.class); // lightweight attributes private String weblogHandle = null; @@ -85,12 +86,11 @@ public WeblogRequest(HttpServletRequest request) } String[] pathElements = path.split("/", 2); - if(!pathElements[0].isBlank()) { + if(StringUtils.isAlphanumeric(pathElements[0])) { this.weblogHandle = pathElements[0]; } else { - // no weblogHandle in path info - throw new InvalidRequestException("not a weblog request, "+ - request.getRequestURL()); + // no or invalid weblogHandle in path info + throw new InvalidRequestException("not a valid weblog request: "+request.getRequestURL()); } // if there is more left of the path info then hold onto it From 91affbd98169c4bd4307ec2390dcff862d12f3cb Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 18 May 2021 02:24:49 +0200 Subject: [PATCH 5/9] Exception handling can be simplified since velocity is now throwing subtypes of VelocityException. --- .../rendering/velocity/VelocityRenderer.java | 55 +++---------------- 1 file changed, 7 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRenderer.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRenderer.java index e348a849a1..3ea45bab61 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRenderer.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRenderer.java @@ -32,8 +32,6 @@ import org.apache.roller.weblogger.ui.rendering.model.UtilitiesModel; import org.apache.velocity.VelocityContext; import org.apache.velocity.context.Context; -import org.apache.velocity.exception.MethodInvocationException; -import org.apache.velocity.exception.ParseErrorException; import org.apache.velocity.exception.ResourceNotFoundException; import org.apache.velocity.exception.VelocityException; @@ -42,11 +40,11 @@ */ public class VelocityRenderer implements Renderer { - private static Log log = LogFactory.getLog(VelocityRenderer.class); + private static final Log log = LogFactory.getLog(VelocityRenderer.class); // the original template we are supposed to render - private Template renderTemplate = null; - private MobileDeviceRepository.DeviceType deviceType = null; + private final Template renderTemplate; + private final MobileDeviceRepository.DeviceType deviceType; // the velocity templates private org.apache.velocity.Template velocityTemplate = null; @@ -77,34 +75,13 @@ public VelocityRenderer(Template template, // failed throw ex; - } catch (ParseErrorException ex) { - // in the case of a parsing error we want to render an - // error page instead so the user knows what was wrong - velocityException = ex; - - // need to lookup error page template - velocityTemplate = RollerVelocity.getTemplate("error-page.vm", - deviceType); - - } catch (MethodInvocationException ex) { - - // in the case of a invocation error we want to render an - // error page instead so the user knows what was wrong - velocityException = ex; - - // need to lookup error page template - velocityTemplate = RollerVelocity.getTemplate("error-page.vm", - deviceType); - } catch (VelocityException ex) { - - // in the case of a parsing error including a macro we want to - // render an error page instead so the user knows what was wrong + // in the case of a velocity error we want to render an + // error page instead so the user knows what was wrong velocityException = ex; // need to lookup error page template - velocityTemplate = RollerVelocity.getTemplate("error-page.vm", - deviceType); + velocityTemplate = RollerVelocity.getTemplate("error-page.vm", deviceType); } catch (Exception ex) { // some kind of generic/unknown exception, dump it to the logs @@ -172,27 +149,9 @@ public void render(Map model, Writer out) log.debug("Rendered [" + renderTemplate.getId() + "] in " + renderTime + " secs"); - } catch (ParseErrorException ex) { - - // in the case of a parsing error including a page we want to render - // an error on the page instead so the user knows what was wrong - velocityException = ex; - - // need to lookup parse error template - renderException(model, out, "error-parse.vm"); - - } catch (MethodInvocationException ex) { - - // in the case of a parsing error including a page we want to render - // an error on the page instead so the user knows what was wrong - velocityException = ex; - - // need to lookup parse error template - renderException(model, out, "error-parse.vm"); - } catch (VelocityException ex) { - // in the case of a parsing error including a macro we want to + // in the case of a velocity error including a page we want to // render an error page instead so the user knows what was wrong velocityException = ex; From d61a0fc8740b101503d07a9b82de288ab127bea1 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 18 May 2021 03:02:04 +0200 Subject: [PATCH 6/9] exception handling / logging. --- .../velocity/RollerResourceLoader.java | 9 ++++---- .../velocity/VelocityRendererFactory.java | 21 ++++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/RollerResourceLoader.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/RollerResourceLoader.java index e81bf06256..4159c6437b 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/RollerResourceLoader.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/RollerResourceLoader.java @@ -46,7 +46,7 @@ */ public class RollerResourceLoader extends ResourceLoader { - private static Log logger = LogFactory.getLog(RollerResourceLoader.class); + private static final Log logger = LogFactory.getLog(RollerResourceLoader.class); @Override public void init(ExtProperties configuration) { @@ -105,15 +105,14 @@ public Reader getResourceReader(String name, String encoding) { } catch (UnsupportedEncodingException uex) { // This should never actually happen. We expect UTF-8 in all JRE // installation. - // This rethrows as a Runtime exception after logging. - logger.error(uex); +// logger.error(uex); throw new RuntimeException(uex); } catch (WebloggerException | ResourceNotFoundException re) { String msg = "RollerResourceLoader Error: " + "database problem trying to load resource " + name; - logger.error(msg, re); - throw new ResourceNotFoundException(msg); +// logger.error(msg, re); + throw new ResourceNotFoundException(msg, re); } } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRendererFactory.java b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRendererFactory.java index 631f573424..62f55d257a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRendererFactory.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/rendering/velocity/VelocityRendererFactory.java @@ -28,44 +28,45 @@ import org.apache.roller.weblogger.ui.rendering.Renderer; import org.apache.roller.weblogger.ui.rendering.RendererFactory; import org.apache.roller.weblogger.ui.rendering.mobile.MobileDeviceRepository; +import org.apache.velocity.exception.ResourceNotFoundException; /** * RendererFactory for Velocity, creates VelocityRenderers. */ public class VelocityRendererFactory implements RendererFactory { - private static Log log = LogFactory.getLog(VelocityRendererFactory.class); + private static final Log log = LogFactory.getLog(VelocityRendererFactory.class); @Override public Renderer getRenderer(Template template, MobileDeviceRepository.DeviceType deviceType) { - Renderer renderer = null; - TemplateRendition tr; + // nothing we can do with null values if (template == null || template.getId() == null) { return null; } - // nothing we can do with null values + TemplateRendition tr; try { tr = template.getTemplateRendition(RenditionType.STANDARD); + if (tr == null) { + return null; + } } catch (WebloggerException e) { return null; } - if (tr == null) { - return null; - } + Renderer renderer = null; if (TemplateLanguage.VELOCITY.equals(tr.getTemplateLanguage())) { // standard velocity template try { renderer = new VelocityRenderer(template, deviceType); + } catch (ResourceNotFoundException ex) { + // allready logged in VelocityRenderer } catch(Exception ex) { - log.error("ERROR creating VelocityRenderer", ex); // some kind of exception so we don't have a renderer - // we do catching/logging in VelocityRenderer constructor - return null; + log.error("ERROR creating VelocityRenderer", ex); } } return renderer; From 54bef887968ee76ec12286f7a25b93f2ee2128c2 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 18 May 2021 03:12:29 +0200 Subject: [PATCH 7/9] added error pages for 400 and 500 errors. --- app/src/main/webapp/WEB-INF/web.xml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/src/main/webapp/WEB-INF/web.xml b/app/src/main/webapp/WEB-INF/web.xml index 295dfaa4c5..bd1a0ab134 100644 --- a/app/src/main/webapp/WEB-INF/web.xml +++ b/app/src/main/webapp/WEB-INF/web.xml @@ -1,8 +1,8 @@ - + xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_4_0.xsd" + version="4.0"> Roller Weblogger @@ -462,11 +462,22 @@ /roller-ui/errors/error.jsp + + 500 + /roller-ui/errors/error.jsp + + 403 /roller-ui/errors/403.jsp + + + 400 + /roller-ui/errors/404.jsp + + 404 /roller-ui/errors/404.jsp From e35a9673dd2ef2639bc05c4e76db769948654722 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Sun, 6 Jun 2021 16:41:56 +0200 Subject: [PATCH 8/9] fix: this should be && not ||. --- .../roller/weblogger/webservices/tagdata/TagDataServlet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java b/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java index 527731953d..a281bcd495 100644 --- a/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java +++ b/app/src/main/java/org/apache/roller/weblogger/webservices/tagdata/TagDataServlet.java @@ -107,7 +107,7 @@ public void doGet( if (request.getParameter("format") != null) { format = request.getParameter("format"); - if(!format.equals("json") || !format.equals("xml")) { + if(!format.equals("json") && !format.equals("xml")) { response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malformed URL"); return; } From 84cd3dea1096b71b6565a45b3575c768e466d09c Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Tue, 22 Jun 2021 02:15:11 +0200 Subject: [PATCH 9/9] escape html in weblog title + remove non alphanumeric chars in tags. --- .../roller/weblogger/ui/struts2/editor/EntryBean.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryBean.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryBean.java index 79f4f9104a..bbc0eab7ea 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryBean.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryBean.java @@ -21,6 +21,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.text.StringEscapeUtils; import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.WeblogEntryManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -28,6 +29,7 @@ import org.apache.roller.weblogger.pojos.WeblogEntry; import org.apache.roller.weblogger.pojos.WeblogEntry.PubStatus; import org.apache.roller.weblogger.pojos.WeblogEntryAttribute; +import org.apache.roller.weblogger.util.Utilities; import java.sql.Timestamp; import java.text.DateFormat; @@ -45,7 +47,7 @@ */ public class EntryBean { - private static Log log = LogFactory.getLog(EntryBean.class); + private static final Log log = LogFactory.getLog(EntryBean.class); private String id = null; private String title = null; @@ -290,12 +292,12 @@ public boolean isScheduled() { public void copyTo(WeblogEntry entry) throws WebloggerException { - entry.setTitle(getTitle()); + entry.setTitle(StringEscapeUtils.escapeHtml4(getTitle())); entry.setStatus(PubStatus.valueOf(getStatus())); entry.setLocale(getLocale()); entry.setSummary(getSummary()); entry.setText(getText()); - entry.setTagsAsString(getTagsAsString()); + entry.setTagsAsString(Utilities.replaceNonAlphanumeric(getTagsAsString(), ' ')); entry.setSearchDescription(getSearchDescription()); // figure out the category selected @@ -337,7 +339,7 @@ public void copyTo(WeblogEntry entry) throws WebloggerException { public void copyFrom(WeblogEntry entry, Locale locale) { setId(entry.getId()); - setTitle(entry.getTitle()); + setTitle(StringEscapeUtils.unescapeHtml4(entry.getTitle())); setLocale(entry.getLocale()); setStatus(entry.getStatus().name()); setSummary(entry.getSummary());