html escaping cleanups.
diff --git a/src/fniki/freenet/filter/WikiContentFilter.java b/src/fniki/freenet/filter/WikiContentFilter.java --- a/src/fniki/freenet/filter/WikiContentFilter.java +++ b/src/fniki/freenet/filter/WikiContentFilter.java @@ -59,8 +59,8 @@ class WikiContentFilter implements Conte * @throws CommentException If the URI is nvalid or unacceptable in some way. */ public String processURI(String uri, String overrideType) throws CommentException { - System.err.println("processURI(0): " + uri + " : " + overrideType); if (!(uri.startsWith(mContainerPrefix) || uri.startsWith(mFproxyPrefix))) { + System.err.println("processURI(0): " + uri + " : " + overrideType); System.err.println("processURI(0): REJECTED URI"); filterTripped(); return null; @@ -78,8 +78,8 @@ class WikiContentFilter implements Conte public String processURI(String uri, String overrideType, boolean noRelative, boolean inline) throws CommentException { // inline is true for images (which we allow mod URI filtering). // noRelative is true if you must return an absolute URI, which we don't allow. - System.err.println("processURI(1): " + uri + " : " + overrideType + " : " + noRelative + " : " + inline); if (noRelative) { + System.err.println("processURI(1): " + uri + " : " + overrideType + " : " + noRelative + " : " + inline); System.err.println("processURI(1): REJECTED URI because of noRelative."); filterTripped(); return null; @@ -168,6 +168,16 @@ class WikiContentFilter implements Conte baos, "text/html", UTF8, this); + + if (status.charset.equals("UTF-8")) { + throw new ServerErrorException("BUG: Generated output with unexpected " + + "character set. But we caught it :-)"); + } + if (!status.mimeType.equals("text/html")) { + throw new ServerErrorException("BUG: Generated output with unexpected " + + "mime type. But we caught it :-)"); + + } return postProcess(new String(baos.toByteArray(), UTF8), html); } catch (UnsafeContentTypeException ucte) { ucte.printStackTrace(); diff --git a/src/fniki/freenet/plugin/Fniki.java b/src/fniki/freenet/plugin/Fniki.java --- a/src/fniki/freenet/plugin/Fniki.java +++ b/src/fniki/freenet/plugin/Fniki.java @@ -55,10 +55,11 @@ public class Fniki implements FredPlugin private WikiApp mWikiApp; private String mContainerPrefix; public void terminate() { - System.err.println("terminating..."); + System.err.println("Fniki plugin terminating..."); } public void runPlugin(PluginRespirator pr) { + System.err.println("Fniki plugin starting..."); try { ArchiveManager archiveManager = new ArchiveManager(); archiveManager.createEmptyArchive(); @@ -77,7 +78,7 @@ public class Fniki implements FredPlugin mWikiApp = wikiApp; } catch (IOException ioe) { - System.err.println("EPIC FAIL!"); + System.err.println("Fniki Plugin EPIC FAIL!"); ioe.printStackTrace(); } } @@ -109,31 +110,33 @@ public class Fniki implements FredPlugin // Then read multipart params if there are any. try { - for (String part : mParent.getParts()) { - if (!allParams.contains(part)) { - continue; + try { + for (String part : mParent.getParts()) { + if (!allParams.contains(part)) { + continue; + } + + String value = new String(mParent.getPartAsBytesFailsafe(part, 64 * 1024), "utf-8"); + mParamTable.put(part, value); + System.err.println("Set multipart Param: " + part + " : " + + mParamTable.get(part)); } + } catch (UnsupportedEncodingException ue) { + // Shouldn't happen. + ue.printStackTrace(); + } - String value = new String(mParent.getPartAsBytesFailsafe(part, 64 * 1024), "utf-8"); - mParamTable.put(part, value); - System.err.println("Set multipart Param: " + part + " : " + - mParamTable.get(part)); + if (!mParamTable.containsKey("action")) { + System.err.println("Forced default action to view"); + mParamTable.put("action", "view"); } - } catch (UnsupportedEncodingException ue) { - // Shouldn't happen. - ue.printStackTrace(); + + if (!mParamTable.containsKey("title")) { + mParamTable.put("title", mPath); + } + } finally { + mParent.freeParts(); } - - if (!mParamTable.containsKey("action")) { - System.err.println("Forced default action to view"); - mParamTable.put("action", "view"); - } - - // DCI: title validation? - if (!mParamTable.containsKey("title")) { - mParamTable.put("title", mPath); - } - mParent.freeParts(); // DCI: test!, put in finally? } PluginQuery(HTTPRequest parent, String path) throws IOException { diff --git a/src/fniki/standalone/FnikiContextHandler.java b/src/fniki/standalone/FnikiContextHandler.java --- a/src/fniki/standalone/FnikiContextHandler.java +++ b/src/fniki/standalone/FnikiContextHandler.java @@ -101,7 +101,6 @@ public class FnikiContextHandler impleme mParamTable.put("action", "view"); } - // DCI: title validation? if (!mParamTable.containsKey("title")) { mParamTable.put("title", mPath); } diff --git a/src/fniki/wiki/HtmlUtils.java b/src/fniki/wiki/HtmlUtils.java --- a/src/fniki/wiki/HtmlUtils.java +++ b/src/fniki/wiki/HtmlUtils.java @@ -84,11 +84,13 @@ public class HtmlUtils { } public static String makeFproxyHref(String fproxyPrefix, String freenetUri) { - // DCI: url encode? use key=? - return fproxyPrefix + freenetUri; + try { + return new URI(fproxyPrefix + "?key=" +freenetUri).toString(); + } catch (URISyntaxException se) { + return "HTML_UTILS_MAKE_FPROXY_HREF_FAILED"; + } } - // DCI: option to convert '_' -> ' ' public static void appendPageLink(String prefix, StringBuilder sb, String name, String action, boolean asTitle) { String title = name; if (asTitle) { @@ -105,7 +107,7 @@ public class HtmlUtils { if (values.size() == 0) { return; } - sb.append(label); + sb.append(escapeHTML(label)); sb.append(": "); List<String> sorted = new ArrayList<String>(values); Collections.sort(sorted); @@ -132,7 +134,7 @@ public class HtmlUtils { " <input type=submit value=\"%s\">" + " <input type=hidden name=\"action\" value=\"%s\">" + "</form>"; - return String.format(fmt, makeHref(fullPath), label, action); + return String.format(fmt, makeHref(fullPath), escapeHTML(label), escapeHTML(action)); } public static String getVersionLink(String prefix, String name, String uri, String action) { @@ -145,25 +147,4 @@ public class HtmlUtils { public static String getVersionLink(String prefix, String name, String uri) { return getVersionLink(prefix, name, uri, "finished"); } - - public static boolean isValidFreenetUri(String link) { - // DCI: do much better! - return (link.startsWith("freenet:CHK@") || - link.startsWith("freenet:SSK@") || - link.startsWith("freenet:USK@")); - } - - public static boolean isValidLocalLink(String link) { - for (int index = 0; index < link.length(); index++) { - char c = link.charAt(index); - if ((c >= '0' && c <= '9') || - (c >= 'a' && c <= 'z') || - (c >= 'A' && c <= 'Z') || - c == '_') { - continue; - } - return false; - } - return true; - } } \ No newline at end of file diff --git a/src/fniki/wiki/QueryBase.java b/src/fniki/wiki/QueryBase.java --- a/src/fniki/wiki/QueryBase.java +++ b/src/fniki/wiki/QueryBase.java @@ -37,7 +37,7 @@ public abstract class QueryBase implemen // MUST contain every parameter used by any ChildContainer. protected final static String PARAMS[] = new String[] { - "action", + "action", "title", "uri", "goto", "savepage", "savetext", "formPassword", diff --git a/src/fniki/wiki/Validations.java b/src/fniki/wiki/Validations.java new file mode 100644 --- /dev/null +++ b/src/fniki/wiki/Validations.java @@ -0,0 +1,61 @@ +/* Validation utility functions + * + * Copyright (C) 2010, 2011 Darrell Karbott + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License as published by the Free Software Foundation; either + * version 2.0 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: djk@isFiaD04zgAgnrEC5XJt1i4IE7AkNPqhBG5bONi6Yks + * + * This file was developed as component of + * "fniki" (a wiki implementation running over Freenet). + */ + +package fniki.wiki; + +public class Validations { + // empty is allowed + public static boolean isAlphaNumOrUnder(String value) { + for (int index = 0; index < value.length(); index++) { + char c = value.charAt(index); + if ((c >= '0' && c <= '9') || + (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + c == '_') { + continue; + } + return false; + } + return true; + } + + // empty is allowed + public static boolean isLowerCaseAlpha(String value) { + for (int index = 0; index < value.length(); index++) { + char c = value.charAt(index); + if (c >= 'a' && c <= 'z') { + continue; + } + return false; + } + return true; + } + + public static boolean isValidFreenetUri(String link) { + // DCI: do much better! + return (link.startsWith("freenet:CHK@") || + link.startsWith("freenet:SSK@") || + link.startsWith("freenet:USK@")); + } +} diff --git a/src/fniki/wiki/WikiApp.java b/src/fniki/wiki/WikiApp.java --- a/src/fniki/wiki/WikiApp.java +++ b/src/fniki/wiki/WikiApp.java @@ -36,6 +36,7 @@ import java.util.Set; import wormarc.FileManifest; import static fniki.wiki.HtmlUtils.*; +import static fniki.wiki.Validations.*; import fniki.wiki.child.AsyncTaskContainer; import fniki.wiki.child.DefaultRedirect; @@ -316,7 +317,7 @@ public class WikiApp implements ChildCon sb.append("</a>"); return; } - if (isValidLocalLink(link[0])) { + if (isAlphaNumOrUnder(link[0])) { // Link to an internal wiki page. sb.append("<a href=\""+ makeHref(mContext.makeLink("/" + link[0].trim())) +"\" rel=\"nofollow\">"); sb.append(escapeHTML(unescapeHTML(link.length>=2 && !isEmpty(link[1].trim())? link[1]:link[0]))); @@ -459,19 +460,21 @@ public class WikiApp implements ChildCon mArchiveManager.setBissName(config.mWikiName); } - // DCI: Think this through. public String makeLink(String containerRelativePath) { // Hacks to find bugs if (!containerRelativePath.startsWith("/")) { containerRelativePath = "/" + containerRelativePath; System.err.println("WikiApp.makeLink -- added leading '/': " + containerRelativePath); + (new RuntimeException("find missing /")).printStackTrace(); + } String full = containerPrefix() + containerRelativePath; while (full.indexOf("//") != -1) { System.err.println("WikiApp.makeLink -- fixing '//': " + full); full = full.replace("//", "/"); + (new RuntimeException("find extra /")).printStackTrace(); } return full; } diff --git a/src/fniki/wiki/child/AsyncTaskContainer.java b/src/fniki/wiki/child/AsyncTaskContainer.java --- a/src/fniki/wiki/child/AsyncTaskContainer.java +++ b/src/fniki/wiki/child/AsyncTaskContainer.java @@ -78,8 +78,6 @@ public abstract class AsyncTaskContainer context.raiseRedirect(context.makeLink("/" + target), "Redirecting..."); } - - // DCI: use a single form? Really ugly. protected void addButtonsHtml(WikiContext context, PrintWriter writer, String confirmTitle, String cancelTitle) { diff --git a/src/fniki/wiki/child/LoadingChangeLog.java b/src/fniki/wiki/child/LoadingChangeLog.java --- a/src/fniki/wiki/child/LoadingChangeLog.java +++ b/src/fniki/wiki/child/LoadingChangeLog.java @@ -65,7 +65,7 @@ public class LoadingChangeLog extends As startTask(); sendRedirect(context, context.getPath()); return "unreachable code"; - } // DCI: ripped out code, need to fix links + } boolean showBuffer = false; String confirmTitle = null; diff --git a/src/fniki/wiki/child/LoadingVersionList.java b/src/fniki/wiki/child/LoadingVersionList.java --- a/src/fniki/wiki/child/LoadingVersionList.java +++ b/src/fniki/wiki/child/LoadingVersionList.java @@ -135,6 +135,7 @@ public class LoadingVersionList extends } } + // Doesn't need escaping. public static String trustString(int value) { if (value == -1) { return "null"; @@ -163,7 +164,7 @@ public class LoadingVersionList extends for (FMSUtil.BISSRecord record : records) { mListHtml.append(String.format(fmt, escapeHTML(record.mFmsId), - record.mDate, + escapeHTML(record.mDate), getVersionLink(mContainerPrefix, "/jfniki/loadarchive", record.mKey), trustString(record.msgTrust()), diff --git a/src/fniki/wiki/child/QueryError.java b/src/fniki/wiki/child/QueryError.java --- a/src/fniki/wiki/child/QueryError.java +++ b/src/fniki/wiki/child/QueryError.java @@ -36,7 +36,6 @@ public class QueryError implements Child public QueryError() {} public String handle(WikiContext context) throws ChildContainerException { - // DCI: force redirect to a default location? context.raiseAccessDenied("Couldn't resolve query or post."); return "unreachable code"; } diff --git a/src/fniki/wiki/child/SettingConfig.java b/src/fniki/wiki/child/SettingConfig.java --- a/src/fniki/wiki/child/SettingConfig.java +++ b/src/fniki/wiki/child/SettingConfig.java @@ -151,7 +151,7 @@ public class SettingConfig implements Mo } mConfig = parseConfigFromPost(context, query, context.getInt("listen_port", 8083), mPrivateSSK); - + try { mConfig.validate(); context.setConfiguration(mConfig); @@ -171,28 +171,30 @@ public class SettingConfig implements Mo } } - // DCI: back over this. escape quotes. public String handle(WikiContext context) throws ChildContainerException { handlePost(context); String href = makeHref(context.makeLink("/fniki/config"), null, null, null, null); + // Html escape CDATA + // http://www.w3.org/TR/html401/types.html#type-cdata return String.format(formTemplate(), - getMsgHtml(), - href, - mConfig.mFcpHost, - mConfig.mFcpPort, - mConfig.mFproxyPrefix, - mConfig.mFmsHost, - mConfig.mFmsPort, - mConfig.mFmsId, - mPublicFmsId, - mConfig.mFmsGroup, - mConfig.mWikiName, - mConfig.mAllowImages ? "checked" : "", + getMsgHtml(), // Already escaped + href, // Not escaped + escapeHTML(mConfig.mFcpHost), + mConfig.mFcpPort, // Integer + escapeHTML(mConfig.mFproxyPrefix), + escapeHTML(mConfig.mFmsHost), + mConfig.mFmsPort, // Integer + escapeHTML(mConfig.mFmsId), + escapeHTML(mPublicFmsId), + escapeHTML(mConfig.mFmsGroup), + escapeHTML(mConfig.mWikiName), + mConfig.mAllowImages ? "checked" : "", // Not escaped // IMPORTANT: Won't work as a plugin without this. - context.getString("form_password", "FORM_PASSWORD_NOT_SET")); + context.getString("form_password", "FORM_PASSWORD_NOT_SET") // Not escaped + ); } public String getMsgHtml() { @@ -296,4 +298,3 @@ public class SettingConfig implements Mo } } -// DCI: include form html and script in comments. diff --git a/src/fniki/wiki/child/WikiContainer.java b/src/fniki/wiki/child/WikiContainer.java --- a/src/fniki/wiki/child/WikiContainer.java +++ b/src/fniki/wiki/child/WikiContainer.java @@ -45,6 +45,7 @@ import fniki.wiki.Query; import wormarc.FileManifest; import fniki.wiki.FreenetWikiTextParser; import static fniki.wiki.HtmlUtils.*; +import static fniki.wiki.Validations.*; import fniki.wiki.WikiApp; import fniki.wiki.WikiContext; import fniki.wiki.WikiTextStorage; @@ -63,8 +64,17 @@ public class WikiContainer implements Ch // a link from a finished task page. e.g. changelog. action = "view"; } + String title = context.getTitle(); + if (!isAlphaNumOrUnder(title)) { + // Titles must be legal page names. + context.raiseAccessDenied("Couldn't work out query."); + } - String title = context.getTitle(); + if (!isLowerCaseAlpha(action)) { + // Illegal action + context.raiseAccessDenied("Couldn't work out query."); + } + Query query = context.getQuery(); if (action.equals("view")) { @@ -129,9 +139,8 @@ public class WikiContainer implements Ch return "unreachable code"; } - private String titleFromName(String name) { - // DCI: html escape - return name.replace("_", " "); // DCI: Much more to it? + private String unescapedTitleFromName(String name) { + return name.replace("_", " "); } private String getPageHtml(WikiContext context, String name) throws IOException { @@ -158,7 +167,7 @@ public class WikiContainer implements Ch "\"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n"); buffer.append("<html xmlns=\"http://www.w3.org/1999/xhtml\">\n"); buffer.append("<head><title>\n"); - buffer.append(escapeHTML(titleFromName(name))); + buffer.append(escapeHTML(unescapedTitleFromName(name))); buffer.append("</title>\n"); buffer.append("<style type=\"text/css\">div.indent{margin-left:20px;} " + "div.center{text-align:center;} " + @@ -167,12 +176,12 @@ public class WikiContainer implements Ch buffer.append("</head>\n"); buffer.append("<body>\n"); buffer.append("<h1>\n"); - buffer.append(escapeHTML(titleFromName(name))); + buffer.append(escapeHTML(unescapedTitleFromName(name))); buffer.append("</h1><hr>\n"); } private String makeLocalLink(WikiContext context, String name, String action, String label) { - String href = makeHref(context.makeLink(name), action, name, null, null); + String href = makeHref(context.makeLink("/" + name), action, name, null, null); return String.format("<a href=\"%s\">%s</a>", href, escapeHTML(label)); } @@ -230,7 +239,7 @@ public class WikiContainer implements Ch buffer.append("\" accept-charset=\"UTF-8\">\n"); buffer.append("<input type=hidden name=\"savepage\" value=\""); - buffer.append(name); // DCI: percent escape? Ok for now because of name checks + buffer.append(escapeHTML(name)); buffer.append("\">\n"); buffer.append("<textarea wrap=\"virtual\" name=\"savetext\" rows=\"17\" cols=\"120\">\n"); @@ -246,7 +255,7 @@ public class WikiContainer implements Ch buffer.append("<input type=hidden name=formPassword value=\""); // IMPORTANT: Required by Freenet Plugin. - buffer.append(context.getString("form_password", "FORM_PASSWORD_NOT_SET")); // DCI: % encode? + buffer.append(context.getString("form_password", "FORM_PASSWORD_NOT_SET")); // Doesn't need escaping. buffer.append("\"/>\n"); buffer.append("<input type=reset value=\"Reset\">\n"); buffer.append("<br></form>");