From 8da9237fdbfa1c5f57bba51b9a7db50743171c61 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 3 Oct 2026 12:46:12 -0400 Subject: [PATCH 1/9] Preserve pasted entry images when publishing --- .../ui/struts2/editor/EntryEdit.java | 206 ++++++++++++++++++ .../roller/weblogger/util/HTMLSanitizer.java | 17 +- .../weblogger/util/InlineImageData.java | 156 +++++++++++++ .../resources/ApplicationResources.properties | 3 + .../roller/weblogger/config/roller.properties | 6 + app/src/main/webapp/themes/basic/weblog.vm | 2 +- .../main/webapp/themes/basicmobile/weblog.vm | 2 +- app/src/main/webapp/themes/fauxcoly/weblog.vm | 3 +- .../main/webapp/themes/frontpage/_header.vm | 2 +- app/src/main/webapp/themes/gaurav/std_head.vm | 2 +- docs/roller-user-guide.adoc | 10 + 11 files changed, 395 insertions(+), 14 deletions(-) create mode 100644 app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java index 8fda6fcbd..d8770a3f1 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java @@ -18,12 +18,18 @@ package org.apache.roller.weblogger.ui.struts2.editor; +import java.io.ByteArrayInputStream; +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collections; import java.util.Date; +import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.UUID; import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.Log; @@ -31,12 +37,17 @@ import org.apache.roller.util.DateUtil; import org.apache.roller.util.RollerConstants; import org.apache.roller.weblogger.WebloggerException; +import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.WebloggerFactory; import org.apache.roller.weblogger.business.WeblogEntryManager; import org.apache.roller.weblogger.business.plugins.PluginManager; import org.apache.roller.weblogger.business.plugins.entry.WeblogEntryPlugin; import org.apache.roller.weblogger.business.search.IndexManager; +import org.apache.roller.weblogger.config.WebloggerConfig; +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; import org.apache.roller.weblogger.pojos.GlobalPermission; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.pojos.MediaFileDirectory; import org.apache.roller.weblogger.pojos.WeblogCategory; import org.apache.roller.weblogger.pojos.WeblogEntry; import org.apache.roller.weblogger.pojos.WeblogEntry.PubStatus; @@ -47,7 +58,10 @@ import org.apache.roller.weblogger.ui.core.plugins.WeblogEntryEditor; import org.apache.roller.weblogger.ui.struts2.util.UIAction; import org.apache.roller.weblogger.util.EnclosureMetadata; +import org.apache.roller.weblogger.util.InlineImageData; import org.apache.roller.weblogger.util.MailUtil; +import org.apache.roller.weblogger.util.RollerMessages; +import org.apache.roller.weblogger.util.RollerMessages.RollerMessage; import org.apache.roller.weblogger.util.cache.CacheManager; import org.apache.struts2.convention.annotation.AllowedMethods; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -200,7 +214,69 @@ String save() { return failedSave(); } + String submittedText = getBean().getText(); + String submittedSummary = getBean().getSummary(); + List createdImages = new ArrayList<>(); + boolean entrySaved = false; try { + Map images = new HashMap<>(); + List textImages = InlineImageData.findSources(submittedText); + List summaryImages = InlineImageData.findSources(submittedSummary); + boolean keepInline = WebloggerConfig.getBooleanProperty( + "weblog.inlineImages.preferInline") + || !WebloggerRuntimeConfig.getBooleanProperty("uploads.enabled") + || !getActionWeblog().hasUserPermission( + getAuthenticatedUser(), WeblogPermission.POST); + long maxUploadBytes = 0; + if (!keepInline && (!textImages.isEmpty() || !summaryImages.isEmpty())) { + maxUploadBytes = (long) (RollerConstants.ONE_MB_IN_BYTES + * new BigDecimal(WebloggerRuntimeConfig.getProperty( + "uploads.file.maxsize")).doubleValue()); + } + if (!validateInlineImages(textImages, images, keepInline, maxUploadBytes) + || !validateInlineImages(summaryImages, images, keepInline, + maxUploadBytes)) { + return failedSave(); + } + if (!images.isEmpty()) { + if (keepInline) { + String inlineText = normalizeInlineSources(submittedText, + textImages); + String inlineSummary = normalizeInlineSources(submittedSummary, + summaryImages); + if (!inlineFieldFits(inlineText, textImages) + || !inlineFieldFits(inlineSummary, summaryImages)) { + return failedSave(); + } + getBean().setText(inlineText); + getBean().setSummary(inlineSummary); + } else { + MediaFileManager mediaManager = WebloggerFactory.getWeblogger() + .getMediaFileManager(); + MediaFileDirectory directory = mediaManager + .getDefaultMediaFileDirectory(getActionWeblog()); + if (directory == null) { + directory = mediaManager.createDefaultMediaFileDirectory( + getActionWeblog()); + } + Map mediaUrls = new HashMap<>(); + getBean().setText(replaceInlineImages(submittedText, + textImages, images, mediaUrls, directory, + mediaManager, createdImages)); + if (!hasActionErrors()) { + getBean().setSummary(replaceInlineImages(submittedSummary, + summaryImages, images, mediaUrls, directory, + mediaManager, createdImages)); + } + if (hasActionErrors()) { + getBean().setText(submittedText); + getBean().setSummary(submittedSummary); + removeCreatedImages(mediaManager, createdImages); + return failedSave(); + } + } + } + WeblogEntryManager weblogEntryManager = WebloggerFactory.getWeblogger() .getWeblogEntryManager(); @@ -264,6 +340,7 @@ String save() { log.debug("Saving entry"); weblogEntryManager.saveWeblogEntry(weblogEntry); WebloggerFactory.getWeblogger().flush(); + entrySaved = true; // notify search of the new entry if (weblogEntry.isPublished()) { @@ -298,12 +375,141 @@ String save() { } catch (Exception e) { log.error("Error saving new entry", e); + if (!entrySaved) { + getBean().setText(submittedText); + getBean().setSummary(submittedSummary); + removeCreatedImages(WebloggerFactory.getWeblogger() + .getMediaFileManager(), createdImages); + } addError("generic.error.check.logs"); } } return failedSave(); } + private boolean validateInlineImages(List sources, + Map images, boolean keepInline, + long maxUploadBytes) { + for (InlineImageData.Source source : sources) { + if (!keepInline && InlineImageData.exceedsUploadLimit( + source.getValue(), maxUploadBytes)) { + addError("weblogEdit.inlineImageUploadTooLarge"); + return false; + } + InlineImageData.Image image = keepInline + ? InlineImageData.parse(source.getValue()) + : InlineImageData.parseForUpload(source.getValue(), + maxUploadBytes); + if (image == null) { + addError("weblogEdit.inlineImageInvalid"); + return false; + } + images.put(source.getValue(), image); + } + return true; + } + + private boolean inlineFieldFits(String html, List sources) { + if (!sources.isEmpty() && html.getBytes(StandardCharsets.UTF_8).length + > InlineImageData.MAX_FIELD_BYTES) { + addError("weblogEdit.inlineImageTooLarge"); + return false; + } + return true; + } + + private String normalizeInlineSources(String html, + List sources) { + if (html == null || sources.isEmpty()) { + return html; + } + StringBuilder result = new StringBuilder(html.length()); + int cursor = 0; + for (InlineImageData.Source source : sources) { + result.append(html, cursor, source.getStart()); + result.append("src=\"").append(source.getValue()).append('"'); + cursor = source.getEnd(); + } + result.append(html, cursor, html.length()); + return result.toString(); + } + + private String replaceInlineImages(String html, + List sources, + Map images, + Map mediaUrls, MediaFileDirectory directory, + MediaFileManager mediaManager, List createdImages) + throws WebloggerException { + if (html == null || sources.isEmpty()) { + return html; + } + StringBuilder result = new StringBuilder(html.length()); + int cursor = 0; + for (InlineImageData.Source source : sources) { + String url = mediaUrls.get(source.getValue()); + if (url == null) { + InlineImageData.Image image = images.get(source.getValue()); + String name = "entry-image-" + UUID.randomUUID() + "." + + image.getExtension(); + RollerMessages errors = new RollerMessages(); + if (!WebloggerFactory.getWeblogger().getFileContentManager() + .canSave(getActionWeblog(), name, image.getContentType(), + image.getBytes().length, errors)) { + addMediaErrors(errors); + return html; + } + MediaFile media = new MediaFile(); + media.setName(name); + media.setWeblog(getActionWeblog()); + media.setDirectory(directory); + media.setLength(image.getBytes().length); + media.setContentType(image.getContentType()); + media.setInputStream(new ByteArrayInputStream(image.getBytes())); + mediaManager.createMediaFile(getActionWeblog(), media, errors); + if (errors.getErrorCount() > 0) { + addMediaErrors(errors); + return html; + } + createdImages.add(media); + url = media.getPermalink(); + mediaUrls.put(source.getValue(), url); + } + result.append(html, cursor, source.getStart()); + result.append("src=\"").append(url).append('"'); + cursor = source.getEnd(); + } + result.append(html, cursor, html.length()); + return result.toString(); + } + + private void addMediaErrors(RollerMessages errors) { + for (Iterator it = errors.getErrors(); it.hasNext();) { + RollerMessage message = it.next(); + String[] args = message.getArgs(); + addError(message.getKey(), args == null + ? Collections.emptyList() : java.util.Arrays.asList(args)); + } + } + + private void removeCreatedImages(MediaFileManager mediaManager, + List createdImages) { + for (MediaFile image : createdImages) { + try { + mediaManager.removeMediaFile(getActionWeblog(), image); + } catch (WebloggerException cleanupError) { + log.warn("Could not remove an image from a failed entry save", cleanupError); + } + } + if (!createdImages.isEmpty()) { + try { + WebloggerFactory.getWeblogger().flush(); + } catch (WebloggerException cleanupError) { + log.warn("Could not flush image cleanup after a failed entry save", + cleanupError); + } + } + } + EnclosureMetadata validateEnclosure() { if (StringUtils.isEmpty(getBean().getEnclosureURL())) { return null; diff --git a/app/src/main/java/org/apache/roller/weblogger/util/HTMLSanitizer.java b/app/src/main/java/org/apache/roller/weblogger/util/HTMLSanitizer.java index 280e07917..56a8db5e7 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/HTMLSanitizer.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/HTMLSanitizer.java @@ -211,7 +211,8 @@ public static SanitizeResult sanitizer(String html, Pattern allowedTags, Pattern } else if (tag.matches("img|embed") && "src".equals(attr)) { // String[] customSchemes = {"http", "https"}; - if (new UrlValidator(customSchemes).isValid(val)) { + if (new UrlValidator(customSchemes).isValid(val) + || ("img".equals(tag) && InlineImageData.parse(val) != null)) { foundURL = true; } else { ret.invalidTags.add(attr + " " + val); @@ -374,7 +375,7 @@ public static SanitizeResult sanitizer(String html, Pattern allowedTags, Pattern private static List tokenize(String html) { List tokens = new ArrayList<>(); int pos = 0; - String token = ""; + StringBuilder token = new StringBuilder(); int len = html.length(); while (pos < len) { char c = html.charAt(pos); @@ -385,11 +386,11 @@ private static List tokenize(String html) { if ("", html); @@ -402,11 +403,11 @@ private static List tokenize(String html) { //store the current token if (token.length() > 0) { - tokens.add(token); + tokens.add(token.toString()); } //clear the token - token = ""; + token.setLength(0); // serch the end of <......> int end = moveToMarkerEnd(pos, ">", html); @@ -414,7 +415,7 @@ private static List tokenize(String html) { pos = end; } else { - token = token + c; + token.append(c); pos++; } @@ -422,7 +423,7 @@ private static List tokenize(String html) { //store the last token if (token.length() > 0) { - tokens.add(token); + tokens.add(token.toString()); } return tokens; diff --git a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java new file mode 100644 index 000000000..c4c463efa --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java @@ -0,0 +1,156 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.roller.weblogger.util; + +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; +import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** Image data URLs accepted in entry content and their locations in HTML. */ +public final class InlineImageData { + + // MySQL's TEXT column holds 65,535 bytes. Leave room for the rest of an entry. + public static final int MAX_FIELD_BYTES = 60000; + + private static final Pattern IMAGE_TAG = Pattern.compile("(?is)]*>"); + private static final Pattern SOURCE_ATTRIBUTE = Pattern.compile( + "(?is)(?]+))"); + private static final Pattern DATA_URL = Pattern.compile( + "(?i)^data:image/(png|jpeg|gif);base64,([a-z0-9+/]+={0,2})$"); + + private InlineImageData() { + } + + public static List findSources(String html) { + List sources = new ArrayList<>(); + if (html == null) { + return sources; + } + Matcher image = IMAGE_TAG.matcher(html); + while (image.find()) { + Matcher source = SOURCE_ATTRIBUTE.matcher(image.group()); + if (!source.find()) { + continue; + } + int group = source.start(1) >= 0 ? 1 : source.start(2) >= 0 ? 2 : 3; + String value = source.group(group); + if (value.trim().toLowerCase(Locale.ROOT).startsWith("data:")) { + sources.add(new Source(image.start() + source.start(), + image.start() + source.end(), value)); + } + } + return sources; + } + + /** Returns null for unsupported, malformed, or oversized image data. */ + public static Image parse(String value) { + return parse(value, true); + } + + /** Entry saves may upload larger images under the configured media limit. */ + public static Image parseForUpload(String value, long maxBytes) { + if (exceedsUploadLimit(value, maxBytes)) { + return null; + } + Image image = parse(value, false); + return image != null && image.bytes.length <= maxBytes ? image : null; + } + + public static boolean exceedsUploadLimit(String value, long maxBytes) { + if (value == null || maxBytes < 0) { + return true; + } + // Base64 expands three bytes to four characters; the prefix is short. + return value.length() > 64 + ((maxBytes + 2) / 3) * 4; + } + + private static Image parse(String value, boolean inline) { + if (value == null || (inline && value.length() > MAX_FIELD_BYTES)) { + return null; + } + Matcher match = DATA_URL.matcher(value); + if (!match.matches()) { + return null; + } + String type = match.group(1).toLowerCase(Locale.ROOT); + try { + byte[] bytes = Base64.getDecoder().decode(match.group(2)); + if (!hasSignature(type, bytes)) { + return null; + } + return new Image(type, bytes); + } catch (IllegalArgumentException invalid) { + return null; + } + } + + private static boolean hasSignature(String type, byte[] bytes) { + if ("png".equals(type)) { + byte[] signature = {(byte) 0x89, 'P', 'N', 'G', 13, 10, 26, 10}; + if (bytes.length < signature.length) { + return false; + } + for (int i = 0; i < signature.length; i++) { + if (bytes[i] != signature[i]) { + return false; + } + } + return true; + } + if ("jpeg".equals(type)) { + return bytes.length >= 3 && bytes[0] == (byte) 0xff + && bytes[1] == (byte) 0xd8 && bytes[2] == (byte) 0xff; + } + return bytes.length >= 6 && bytes[0] == 'G' && bytes[1] == 'I' + && bytes[2] == 'F' && bytes[3] == '8' + && (bytes[4] == '7' || bytes[4] == '9') && bytes[5] == 'a'; + } + + public static final class Source { + private final int start; + private final int end; + private final String value; + + private Source(int start, int end, String value) { + this.start = start; + this.end = end; + this.value = value; + } + + public int getStart() { return start; } + public int getEnd() { return end; } + public String getValue() { return value; } + } + + public static final class Image { + private final String type; + private final byte[] bytes; + + private Image(String type, byte[] bytes) { + this.type = type; + this.bytes = bytes; + } + + public String getType() { return type; } + public byte[] getBytes() { return bytes; } + public String getExtension() { return "jpeg".equals(type) ? "jpg" : type; } + public String getContentType() { return "image/" + type; } + } +} diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 1aba85d74..c10d10ff3 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -1554,6 +1554,9 @@ weblogEdit.draft=Draft weblogEdit.draftEntries=Recent Drafts weblogEdit.deleteEntry=Delete Entry weblogEdit.insertMediaFile=Insert Media File +weblogEdit.inlineImageInvalid=This entry contains an unsupported or invalid embedded image. Use a PNG, JPEG or GIF image, or insert a media file. +weblogEdit.inlineImageTooLarge=This entry has too much embedded image data. Resize the image or ask an administrator to enable media uploads. +weblogEdit.inlineImageUploadTooLarge=This embedded image exceeds the site's media upload size limit. Resize it before saving. weblogEdit.fullPreviewMode=Full Preview weblogEdit.locale=Language weblogEdit.pendingEntries=Pending Entries diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties index cdb7d5252..1e30283c1 100644 --- a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties +++ b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties @@ -336,6 +336,12 @@ securelogin.enabled=false # With this settings, all users will have HTML posts sanitized. weblogAdminsUntrusted=true +# Upload pasted PNG, JPEG and GIF entry images as media when possible. +# Set true to retain validated images inline even when uploads are available. +# Inline images are used automatically if uploads are disabled or the author +# cannot upload media. Inline entry fields are limited to 60,000 UTF-8 bytes. +weblog.inlineImages.preferInline=false + # Empty value used for passphrase in roller_user table when LDAP or CMA used; # openid presently generates a random (long) password string instead. users.passwords.externalAuthValue= diff --git a/app/src/main/webapp/themes/basic/weblog.vm b/app/src/main/webapp/themes/basic/weblog.vm index 17f52ad2d..1652ab7a2 100644 --- a/app/src/main/webapp/themes/basic/weblog.vm +++ b/app/src/main/webapp/themes/basic/weblog.vm @@ -3,7 +3,7 @@ - + $model.weblog.name #showAutodiscoveryLinks($model.weblog) #showAnalyticsTrackingCode($model.weblog) diff --git a/app/src/main/webapp/themes/basicmobile/weblog.vm b/app/src/main/webapp/themes/basicmobile/weblog.vm index 88504abad..8cb6de293 100644 --- a/app/src/main/webapp/themes/basicmobile/weblog.vm +++ b/app/src/main/webapp/themes/basicmobile/weblog.vm @@ -3,7 +3,7 @@ - + $model.weblog.name #showAutodiscoveryLinks($model.weblog) #showAnalyticsTrackingCode($model.weblog) diff --git a/app/src/main/webapp/themes/fauxcoly/weblog.vm b/app/src/main/webapp/themes/fauxcoly/weblog.vm index bee525959..b8dbd3171 100644 --- a/app/src/main/webapp/themes/fauxcoly/weblog.vm +++ b/app/src/main/webapp/themes/fauxcoly/weblog.vm @@ -3,7 +3,7 @@ - + #includeTemplate($model.weblog "standard_head") $model.weblog.name: $model.weblog.tagline #showAutodiscoveryLinks($model.weblog) @@ -117,4 +117,3 @@ Click the link below to subscribe via your favorite feed reader:

- diff --git a/app/src/main/webapp/themes/frontpage/_header.vm b/app/src/main/webapp/themes/frontpage/_header.vm index 0e4e77005..5c9cdc5bb 100644 --- a/app/src/main/webapp/themes/frontpage/_header.vm +++ b/app/src/main/webapp/themes/frontpage/_header.vm @@ -3,7 +3,7 @@ - + $model.weblog.name #showAutodiscoveryLinks($model.weblog) diff --git a/app/src/main/webapp/themes/gaurav/std_head.vm b/app/src/main/webapp/themes/gaurav/std_head.vm index 94318dcc7..e68bba6e7 100755 --- a/app/src/main/webapp/themes/gaurav/std_head.vm +++ b/app/src/main/webapp/themes/gaurav/std_head.vm @@ -1,5 +1,5 @@ - + #if ($model.permalink == false) #else diff --git a/docs/roller-user-guide.adoc b/docs/roller-user-guide.adoc index 050b4ce1a..0a6a0253f 100644 --- a/docs/roller-user-guide.adoc +++ b/docs/roller-user-guide.adoc @@ -402,6 +402,16 @@ image::user-guide-11-blogroll.png[] === Uploading images and other files to your weblog +When you paste or drag a local PNG, JPEG or GIF image into the rich text +editor, Roller stores it as a media file when uploads are available to you. +If uploads are unavailable, Roller keeps the image in the entry. Inline images +are limited to 60,000 UTF-8 bytes per content or summary field, including +the surrounding text. If an older entry contains embedded images, saving it +again applies the same rule. An administrator can set +`weblog.inlineImages.preferInline=true` in `roller-custom.properties` to +prefer inline images even when uploads are available. Custom themes with a +Content Security Policy must allow `data:` in `img-src` to show inline images. + If you’d like to upload images or other files for use in your weblog, go to your weblog’s *Create & Edit -> Media Files* page. From there you can upload files, browse and search files. You can also manage your files, From 97db6d2360731e028137346fe5e52c1870949c2b Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 3 Oct 2026 17:17:53 -0400 Subject: [PATCH 2/9] Make the inline image field limit configurable Add weblog.inlineImages.maxFieldBytes (default 60000). Document the database column sizes that bound it. --- .../ui/struts2/editor/EntryEdit.java | 2 +- .../weblogger/util/InlineImageData.java | 31 +++++++- .../roller/weblogger/config/roller.properties | 9 ++- .../weblogger/util/InlineImageDataTest.java | 71 +++++++++++++++++++ docs/roller-user-guide.adoc | 13 ++-- 5 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java index d8770a3f1..3a8690013 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java @@ -411,7 +411,7 @@ private boolean validateInlineImages(List sources, private boolean inlineFieldFits(String html, List sources) { if (!sources.isEmpty() && html.getBytes(StandardCharsets.UTF_8).length - > InlineImageData.MAX_FIELD_BYTES) { + > InlineImageData.maxFieldBytes()) { addError("weblogEdit.inlineImageTooLarge"); return false; } diff --git a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java index c4c463efa..6634c3abf 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java @@ -23,11 +23,19 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.config.WebloggerConfig; + /** Image data URLs accepted in entry content and their locations in HTML. */ public final class InlineImageData { + private static final Log log = LogFactory.getLog(InlineImageData.class); + + static final String MAX_FIELD_BYTES_PROPERTY = "weblog.inlineImages.maxFieldBytes"; + // MySQL's TEXT column holds 65,535 bytes. Leave room for the rest of an entry. - public static final int MAX_FIELD_BYTES = 60000; + static final int DEFAULT_MAX_FIELD_BYTES = 60000; private static final Pattern IMAGE_TAG = Pattern.compile("(?is)]*>"); private static final Pattern SOURCE_ATTRIBUTE = Pattern.compile( @@ -38,6 +46,25 @@ public final class InlineImageData { private InlineImageData() { } + /** Largest inline content or summary field, in UTF-8 bytes. */ + public static int maxFieldBytes() { + String value = WebloggerConfig.getProperty(MAX_FIELD_BYTES_PROPERTY); + if (value == null || value.trim().isEmpty()) { + return DEFAULT_MAX_FIELD_BYTES; + } + try { + int max = Integer.parseInt(value.trim()); + if (max > 0) { + return max; + } + } catch (NumberFormatException invalid) { + // fall through to the default + } + log.warn("Ignoring invalid " + MAX_FIELD_BYTES_PROPERTY + " value '" + value + + "'; using " + DEFAULT_MAX_FIELD_BYTES); + return DEFAULT_MAX_FIELD_BYTES; + } + public static List findSources(String html) { List sources = new ArrayList<>(); if (html == null) { @@ -82,7 +109,7 @@ public static boolean exceedsUploadLimit(String value, long maxBytes) { } private static Image parse(String value, boolean inline) { - if (value == null || (inline && value.length() > MAX_FIELD_BYTES)) { + if (value == null || (inline && value.length() > maxFieldBytes())) { return null; } Matcher match = DATA_URL.matcher(value); diff --git a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties index 1e30283c1..9a2af878a 100644 --- a/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties +++ b/app/src/main/resources/org/apache/roller/weblogger/config/roller.properties @@ -339,9 +339,16 @@ weblogAdminsUntrusted=true # Upload pasted PNG, JPEG and GIF entry images as media when possible. # Set true to retain validated images inline even when uploads are available. # Inline images are used automatically if uploads are disabled or the author -# cannot upload media. Inline entry fields are limited to 60,000 UTF-8 bytes. +# cannot upload media. weblog.inlineImages.preferInline=false +# Largest entry content or summary field, in UTF-8 bytes, that may hold inline +# images. The default fits MySQL's TEXT column (65,535 bytes). Derby and DB2 +# columns hold 102,400 characters. On MySQL, alter weblogentry.text and +# weblogentry.summary to MEDIUMTEXT before raising this value; PostgreSQL, +# Oracle and SQL Server columns need no change. +weblog.inlineImages.maxFieldBytes=60000 + # Empty value used for passphrase in roller_user table when LDAP or CMA used; # openid presently generates a random (long) password string instead. users.passwords.externalAuthValue= diff --git a/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java b/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java new file mode 100644 index 000000000..7ced3efb2 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.roller.weblogger.util; + +import org.apache.roller.weblogger.config.WebloggerConfig; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mockStatic; + +class InlineImageDataTest { + + // 1x1 transparent PNG + private static final String PNG = "data:image/png;base64," + + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="; + + @Test + void fieldLimitDefaultsWhenUnset() { + assertEquals(InlineImageData.DEFAULT_MAX_FIELD_BYTES, withLimit(null)); + } + + @Test + void fieldLimitFollowsTheSetting() { + assertEquals(250000, withLimit("250000")); + } + + @Test + void invalidFieldLimitFallsBackToTheDefault() { + assertEquals(InlineImageData.DEFAULT_MAX_FIELD_BYTES, withLimit("big")); + assertEquals(InlineImageData.DEFAULT_MAX_FIELD_BYTES, withLimit("0")); + assertEquals(InlineImageData.DEFAULT_MAX_FIELD_BYTES, withLimit("-1")); + } + + @Test + void inlineImagesRespectTheFieldLimit() { + try (MockedStatic config = mockStatic(WebloggerConfig.class)) { + config.when(() -> WebloggerConfig.getProperty( + InlineImageData.MAX_FIELD_BYTES_PROPERTY)).thenReturn("20"); + assertNull(InlineImageData.parse(PNG)); + + config.when(() -> WebloggerConfig.getProperty( + InlineImageData.MAX_FIELD_BYTES_PROPERTY)).thenReturn("1000"); + assertNotNull(InlineImageData.parse(PNG)); + } + } + + private static int withLimit(String value) { + try (MockedStatic config = mockStatic(WebloggerConfig.class)) { + config.when(() -> WebloggerConfig.getProperty( + InlineImageData.MAX_FIELD_BYTES_PROPERTY)).thenReturn(value); + return InlineImageData.maxFieldBytes(); + } + } +} diff --git a/docs/roller-user-guide.adoc b/docs/roller-user-guide.adoc index 0a6a0253f..86401d5ea 100644 --- a/docs/roller-user-guide.adoc +++ b/docs/roller-user-guide.adoc @@ -404,12 +404,15 @@ image::user-guide-11-blogroll.png[] When you paste or drag a local PNG, JPEG or GIF image into the rich text editor, Roller stores it as a media file when uploads are available to you. -If uploads are unavailable, Roller keeps the image in the entry. Inline images -are limited to 60,000 UTF-8 bytes per content or summary field, including -the surrounding text. If an older entry contains embedded images, saving it -again applies the same rule. An administrator can set +If uploads are unavailable, Roller keeps the image in the entry. By default, +a content or summary field with inline images is limited to 60,000 UTF-8 +bytes, including the surrounding text. If an older entry contains embedded +images, saving it again applies the same rule. An administrator can set `weblog.inlineImages.preferInline=true` in `roller-custom.properties` to -prefer inline images even when uploads are available. Custom themes with a +prefer inline images even when uploads are available, and can change the +limit with `weblog.inlineImages.maxFieldBytes`. On MySQL, alter the +`weblogentry.text` and `weblogentry.summary` columns to `MEDIUMTEXT` before +raising the limit, because a `TEXT` column holds only 65,535 bytes. Custom themes with a Content Security Policy must allow `data:` in `img-src` to show inline images. If you’d like to upload images or other files for use in your weblog, go From ff6654568c9bc54effa59a964cb9d50fa8b3a1fe Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 3 Oct 2026 17:25:46 -0400 Subject: [PATCH 3/9] Fix inline image parsing and add tests Stop an img tag match at the next '<' so scanning stays linear, and read img attributes in order so src text inside another attribute's quoted value is not taken for the source. Move entry image handling into prepareInlineImages() so it can be tested, add tests for parsing, the sanitizer and the editor save paths, and add the 6.1.7 change log entry. --- CHANGES.md | 16 ++ .../ui/struts2/editor/EntryEdit.java | 127 ++++++----- .../weblogger/util/InlineImageData.java | 77 ++++++- .../editor/EntryEditInlineImagesTest.java | 215 ++++++++++++++++++ .../util/HTMLSanitizerInlineImageTest.java | 50 ++++ .../weblogger/util/InlineImageDataTest.java | 90 +++++++- 6 files changed, 508 insertions(+), 67 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/util/HTMLSanitizerInlineImageTest.java diff --git a/CHANGES.md b/CHANGES.md index 2f4ad53f8..d963f8fa7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,21 @@ # Apache Roller — Changes +## 6.1.7 + +### Improvements + +- **Pasted entry images are kept when you publish** + ([ROL-2184](https://issues.apache.org/jira/browse/ROL-2184)). PNG, JPEG and + GIF images pasted or dragged into the rich text editor are saved as media + files when the author can upload. Otherwise they are kept inline. + - `weblog.inlineImages.preferInline=true` keeps images inline even when + uploads are available. + - `weblog.inlineImages.maxFieldBytes` (default 60000) limits a content or + summary field that has inline images. On MySQL, change the entry columns to + `MEDIUMTEXT` before you raise it. + - Bundled themes allow `data:` images. Custom themes with their own Content + Security Policy need `data:` in `img-src`. + ## 6.1.6 Initial installation now requires a one-time, cryptographically secure setup token printed to the server log. Bootstrap access closes as soon as setup finishes — when the first administrator is created on a new site, or when the database upgrade completes on an existing one. diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java index 3a8690013..f2739860a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java @@ -219,63 +219,9 @@ String save() { List createdImages = new ArrayList<>(); boolean entrySaved = false; try { - Map images = new HashMap<>(); - List textImages = InlineImageData.findSources(submittedText); - List summaryImages = InlineImageData.findSources(submittedSummary); - boolean keepInline = WebloggerConfig.getBooleanProperty( - "weblog.inlineImages.preferInline") - || !WebloggerRuntimeConfig.getBooleanProperty("uploads.enabled") - || !getActionWeblog().hasUserPermission( - getAuthenticatedUser(), WeblogPermission.POST); - long maxUploadBytes = 0; - if (!keepInline && (!textImages.isEmpty() || !summaryImages.isEmpty())) { - maxUploadBytes = (long) (RollerConstants.ONE_MB_IN_BYTES - * new BigDecimal(WebloggerRuntimeConfig.getProperty( - "uploads.file.maxsize")).doubleValue()); - } - if (!validateInlineImages(textImages, images, keepInline, maxUploadBytes) - || !validateInlineImages(summaryImages, images, keepInline, - maxUploadBytes)) { + if (!prepareInlineImages(createdImages)) { return failedSave(); } - if (!images.isEmpty()) { - if (keepInline) { - String inlineText = normalizeInlineSources(submittedText, - textImages); - String inlineSummary = normalizeInlineSources(submittedSummary, - summaryImages); - if (!inlineFieldFits(inlineText, textImages) - || !inlineFieldFits(inlineSummary, summaryImages)) { - return failedSave(); - } - getBean().setText(inlineText); - getBean().setSummary(inlineSummary); - } else { - MediaFileManager mediaManager = WebloggerFactory.getWeblogger() - .getMediaFileManager(); - MediaFileDirectory directory = mediaManager - .getDefaultMediaFileDirectory(getActionWeblog()); - if (directory == null) { - directory = mediaManager.createDefaultMediaFileDirectory( - getActionWeblog()); - } - Map mediaUrls = new HashMap<>(); - getBean().setText(replaceInlineImages(submittedText, - textImages, images, mediaUrls, directory, - mediaManager, createdImages)); - if (!hasActionErrors()) { - getBean().setSummary(replaceInlineImages(submittedSummary, - summaryImages, images, mediaUrls, directory, - mediaManager, createdImages)); - } - if (hasActionErrors()) { - getBean().setText(submittedText); - getBean().setSummary(submittedSummary); - removeCreatedImages(mediaManager, createdImages); - return failedSave(); - } - } - } WeblogEntryManager weblogEntryManager = WebloggerFactory.getWeblogger() .getWeblogEntryManager(); @@ -387,6 +333,77 @@ String save() { return failedSave(); } + /** + * Uploads data images in the submitted text and summary as media files, or + * keeps them inline when uploads are unavailable. Adds an action error and + * returns false if the entry cannot be saved. Media files it creates are + * added to createdImages so a later failure can remove them. + */ + // Package-private so EntryEditInlineImagesTest can drive it directly. + boolean prepareInlineImages(List createdImages) + throws WebloggerException { + String submittedText = getBean().getText(); + String submittedSummary = getBean().getSummary(); + Map images = new HashMap<>(); + List textImages = InlineImageData.findSources(submittedText); + List summaryImages = InlineImageData.findSources(submittedSummary); + boolean keepInline = WebloggerConfig.getBooleanProperty( + "weblog.inlineImages.preferInline") + || !WebloggerRuntimeConfig.getBooleanProperty("uploads.enabled") + || !getActionWeblog().hasUserPermission( + getAuthenticatedUser(), WeblogPermission.POST); + long maxUploadBytes = 0; + if (!keepInline && (!textImages.isEmpty() || !summaryImages.isEmpty())) { + maxUploadBytes = (long) (RollerConstants.ONE_MB_IN_BYTES + * new BigDecimal(WebloggerRuntimeConfig.getProperty( + "uploads.file.maxsize")).doubleValue()); + } + if (!validateInlineImages(textImages, images, keepInline, maxUploadBytes) + || !validateInlineImages(summaryImages, images, keepInline, + maxUploadBytes)) { + return false; + } + if (!images.isEmpty()) { + if (keepInline) { + String inlineText = normalizeInlineSources(submittedText, + textImages); + String inlineSummary = normalizeInlineSources(submittedSummary, + summaryImages); + if (!inlineFieldFits(inlineText, textImages) + || !inlineFieldFits(inlineSummary, summaryImages)) { + return false; + } + getBean().setText(inlineText); + getBean().setSummary(inlineSummary); + } else { + MediaFileManager mediaManager = WebloggerFactory.getWeblogger() + .getMediaFileManager(); + MediaFileDirectory directory = mediaManager + .getDefaultMediaFileDirectory(getActionWeblog()); + if (directory == null) { + directory = mediaManager.createDefaultMediaFileDirectory( + getActionWeblog()); + } + Map mediaUrls = new HashMap<>(); + getBean().setText(replaceInlineImages(submittedText, + textImages, images, mediaUrls, directory, + mediaManager, createdImages)); + if (!hasActionErrors()) { + getBean().setSummary(replaceInlineImages(submittedSummary, + summaryImages, images, mediaUrls, directory, + mediaManager, createdImages)); + } + if (hasActionErrors()) { + getBean().setText(submittedText); + getBean().setSummary(submittedSummary); + removeCreatedImages(mediaManager, createdImages); + return false; + } + } + } + return true; + } + private boolean validateInlineImages(List sources, Map images, boolean keepInline, long maxUploadBytes) { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java index 6634c3abf..7e98a191c 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java @@ -37,9 +37,7 @@ public final class InlineImageData { // MySQL's TEXT column holds 65,535 bytes. Leave room for the rest of an entry. static final int DEFAULT_MAX_FIELD_BYTES = 60000; - private static final Pattern IMAGE_TAG = Pattern.compile("(?is)]*>"); - private static final Pattern SOURCE_ATTRIBUTE = Pattern.compile( - "(?is)(?]+))"); + private static final Pattern IMAGE_TAG = Pattern.compile("(?is)]*>"); private static final Pattern DATA_URL = Pattern.compile( "(?i)^data:image/(png|jpeg|gif);base64,([a-z0-9+/]+={0,2})$"); @@ -72,18 +70,75 @@ public static List findSources(String html) { } Matcher image = IMAGE_TAG.matcher(html); while (image.find()) { - Matcher source = SOURCE_ATTRIBUTE.matcher(image.group()); - if (!source.find()) { + Source source = findSource(image.group(), image.start()); + if (source != null + && source.value.trim().toLowerCase(Locale.ROOT).startsWith("data:")) { + sources.add(source); + } + } + return sources; + } + + /** + * Returns the first src attribute of an img tag. Attributes are read in + * order, so text inside another attribute's quoted value is never taken + * for a src attribute. + */ + private static Source findSource(String tag, int offset) { + int length = tag.length(); + int i = "') { + return null; + } + int nameStart = i; + while (i < length && !isNameEnd(tag.charAt(i))) { + i++; + } + String name = tag.substring(nameStart, i); + int afterName = i; + while (i < length && Character.isWhitespace(tag.charAt(i))) { + i++; + } + if (i >= length || tag.charAt(i) != '=') { + i = afterName; continue; } - int group = source.start(1) >= 0 ? 1 : source.start(2) >= 0 ? 2 : 3; - String value = source.group(group); - if (value.trim().toLowerCase(Locale.ROOT).startsWith("data:")) { - sources.add(new Source(image.start() + source.start(), - image.start() + source.end(), value)); + i++; + while (i < length && Character.isWhitespace(tag.charAt(i))) { + i++; + } + String value; + if (i < length && (tag.charAt(i) == '"' || tag.charAt(i) == '\'')) { + char quote = tag.charAt(i); + int close = tag.indexOf(quote, i + 1); + if (close < 0) { + return null; + } + value = tag.substring(i + 1, close); + i = close + 1; + } else { + int valueStart = i; + while (i < length && !Character.isWhitespace(tag.charAt(i)) + && tag.charAt(i) != '>') { + i++; + } + value = tag.substring(valueStart, i); + } + if ("src".equalsIgnoreCase(name)) { + return new Source(offset + nameStart, offset + i, value); } } - return sources; + return null; + } + + private static boolean isNameEnd(char c) { + return Character.isWhitespace(c) || c == '=' || c == '>' || c == '/'; } /** Returns null for unsupported, malformed, or oversized image data. */ diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java new file mode 100644 index 000000000..31bee5f64 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java @@ -0,0 +1,215 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.roller.weblogger.ui.struts2.editor; + +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; + +import org.apache.roller.weblogger.business.FileContentManager; +import org.apache.roller.weblogger.business.MediaFileManager; +import org.apache.roller.weblogger.business.URLStrategy; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.config.WebloggerConfig; +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.apache.roller.weblogger.pojos.MediaFile; +import org.apache.roller.weblogger.pojos.MediaFileDirectory; +import org.apache.roller.weblogger.pojos.User; +import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.pojos.WeblogPermission; +import org.apache.roller.weblogger.util.RollerMessages; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class EntryEditInlineImagesTest { + + private static final String PNG = "data:image/png;base64," + + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="; + + private MockedStatic config; + private MockedStatic runtimeConfig; + private MockedStatic factory; + private MediaFileManager mediaManager; + private FileContentManager contentManager; + private Weblog weblog; + private EntryEdit action; + private final List created = new ArrayList<>(); + + @BeforeEach + void setUp() throws Exception { + config = mockStatic(WebloggerConfig.class); + runtimeConfig = mockStatic(WebloggerRuntimeConfig.class); + factory = mockStatic(WebloggerFactory.class); + runtimeConfig.when(() -> WebloggerRuntimeConfig.getBooleanProperty("uploads.enabled")) + .thenReturn(true); + runtimeConfig.when(() -> WebloggerRuntimeConfig.getProperty("uploads.file.maxsize")) + .thenReturn("1"); + + mediaManager = mock(MediaFileManager.class); + contentManager = mock(FileContentManager.class); + URLStrategy urls = mock(URLStrategy.class); + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getMediaFileManager()).thenReturn(mediaManager); + when(weblogger.getFileContentManager()).thenReturn(contentManager); + when(weblogger.getUrlStrategy()).thenReturn(urls); + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + when(urls.getMediaFileURL(any(), anyString(), anyBoolean())) + .thenAnswer(call -> "https://blog.example/media/" + call.getArgument(1)); + when(mediaManager.getDefaultMediaFileDirectory(any())) + .thenReturn(new MediaFileDirectory()); + when(contentManager.canSave(any(), anyString(), anyString(), anyLong(), any())) + .thenReturn(true); + + weblog = mock(Weblog.class); + when(weblog.hasUserPermission(any(), eq(WeblogPermission.POST))).thenReturn(true); + + action = spy(new EntryEdit()); + doAnswer(call -> call.getArgument(0)).when(action).getText(anyString()); + doAnswer(call -> call.getArgument(0)).when(action).getText(anyString(), anyList()); + action.setActionWeblog(weblog); + action.setAuthenticatedUser(new User()); + } + + @AfterEach + void tearDown() { + factory.close(); + runtimeConfig.close(); + config.close(); + } + + @Test + void uploadsEachDistinctImageOnceAndRewritesBothFields() throws Exception { + action.getBean().setText("

x

"); + action.getBean().setSummary("a"); + + assertTrue(action.prepareInlineImages(created)); + + assertEquals(1, created.size()); + verify(mediaManager, times(1)).createMediaFile(eq(weblog), any(), any()); + String url = "https://blog.example/media/" + created.get(0).getId(); + assertEquals("

x

", action.getBean().getText()); + assertEquals("a", action.getBean().getSummary()); + assertEquals("image/png", created.get(0).getContentType()); + } + + @Test + void keepsImagesInlineWhenTheAuthorCannotUpload() throws Exception { + when(weblog.hasUserPermission(any(), eq(WeblogPermission.POST))).thenReturn(false); + action.getBean().setText(""); + + assertTrue(action.prepareInlineImages(created)); + + assertEquals("", action.getBean().getText()); + verify(mediaManager, never()).createMediaFile(any(), any(), any()); + } + + @Test + void keepsImagesInlineWhenPreferred() throws Exception { + config.when(() -> WebloggerConfig.getBooleanProperty("weblog.inlineImages.preferInline")) + .thenReturn(true); + action.getBean().setText(""); + + assertTrue(action.prepareInlineImages(created)); + + assertEquals("", action.getBean().getText()); + verify(mediaManager, never()).createMediaFile(any(), any(), any()); + } + + @Test + void refusesInlineFieldsOverTheLimit() throws Exception { + runtimeConfig.when(() -> WebloggerRuntimeConfig.getBooleanProperty("uploads.enabled")) + .thenReturn(false); + config.when(() -> WebloggerConfig.getProperty("weblog.inlineImages.maxFieldBytes")) + .thenReturn("150"); + String text = "

" + "x".repeat(100) + "

"; + action.getBean().setText(text); + + assertFalse(action.prepareInlineImages(created)); + + assertTrue(action.getActionErrors().contains("weblogEdit.inlineImageTooLarge")); + assertEquals(text, action.getBean().getText()); + } + + @Test + void refusesInvalidImageData() throws Exception { + String text = "".getBytes()) + "\">"; + action.getBean().setText(text); + + assertFalse(action.prepareInlineImages(created)); + + assertTrue(action.getActionErrors().contains("weblogEdit.inlineImageInvalid")); + assertEquals(text, action.getBean().getText()); + verify(mediaManager, never()).createMediaFile(any(), any(), any()); + } + + @Test + void refusesImagesOverTheUploadLimit() throws Exception { + runtimeConfig.when(() -> WebloggerRuntimeConfig.getProperty("uploads.file.maxsize")) + .thenReturn("0.00001"); + action.getBean().setText(""); + + assertFalse(action.prepareInlineImages(created)); + + assertTrue(action.getActionErrors().contains("weblogEdit.inlineImageUploadTooLarge")); + verify(mediaManager, never()).createMediaFile(any(), any(), any()); + } + + @Test + void refusedUploadRestoresTheFieldsAndRemovesEarlierUploads() throws Exception { + String gif = "data:image/gif;base64," + + Base64.getEncoder().encodeToString("GIF89a!".getBytes()); + when(contentManager.canSave(any(), anyString(), eq("image/gif"), anyLong(), any())) + .thenAnswer(call -> { + RollerMessages errors = call.getArgument(4); + errors.addError("error.upload.forbiddenFile"); + return false; + }); + String text = ""; + String summary = ""; + action.getBean().setText(text); + action.getBean().setSummary(summary); + + assertFalse(action.prepareInlineImages(created)); + + assertEquals(text, action.getBean().getText()); + assertEquals(summary, action.getBean().getSummary()); + assertEquals(1, created.size()); + verify(mediaManager).removeMediaFile(weblog, created.get(0)); + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/util/HTMLSanitizerInlineImageTest.java b/app/src/test/java/org/apache/roller/weblogger/util/HTMLSanitizerInlineImageTest.java new file mode 100644 index 000000000..15b1e7326 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/util/HTMLSanitizerInlineImageTest.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.roller.weblogger.util; + +import java.util.Base64; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class HTMLSanitizerInlineImageTest { + + @Test + void keepsValidatedImageData() { + String html = HTMLSanitizer.sanitize(""); + assertTrue(html.contains(InlineImageDataTest.PNG), html); + } + + @Test + void removesOtherDataUrls() { + String svg = "data:image/svg+xml;base64," + + Base64.getEncoder().encodeToString("".getBytes()); + assertFalse(HTMLSanitizer.sanitize("").contains("data:")); + assertFalse(HTMLSanitizer.sanitize( + "").contains("data:")); + assertFalse(HTMLSanitizer.sanitize( + "").contains("data:")); + } + + @Test + void removesImageDataOutsideImgTags() { + assertFalse(HTMLSanitizer.sanitize( + "").contains("data:")); + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java b/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java index 7ced3efb2..3b1deb245 100644 --- a/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java @@ -16,6 +16,10 @@ */ package org.apache.roller.weblogger.util; +import java.time.Duration; +import java.util.Base64; +import java.util.List; + import org.apache.roller.weblogger.config.WebloggerConfig; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; @@ -23,14 +27,98 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mockStatic; class InlineImageDataTest { // 1x1 transparent PNG - private static final String PNG = "data:image/png;base64," + static final String PNG = "data:image/png;base64," + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="; + static String dataUrl(String type, byte... bytes) { + return "data:image/" + type + ";base64," + Base64.getEncoder().encodeToString(bytes); + } + + @Test + void acceptsPngJpegAndGifWithMatchingSignatures() { + InlineImageData.Image png = InlineImageData.parse(PNG); + assertNotNull(png); + assertEquals("image/png", png.getContentType()); + assertEquals("png", png.getExtension()); + + InlineImageData.Image jpeg = InlineImageData.parse( + dataUrl("jpeg", (byte) 0xff, (byte) 0xd8, (byte) 0xff, (byte) 0xe0)); + assertNotNull(jpeg); + assertEquals("jpg", jpeg.getExtension()); + + assertNotNull(InlineImageData.parse(dataUrl("gif", "GIF89a!".getBytes()))); + assertNotNull(InlineImageData.parse(dataUrl("gif", "GIF87a!".getBytes()))); + } + + @Test + void refusesOtherTypesAndMismatchedOrMalformedData() { + assertNull(InlineImageData.parse("data:image/svg+xml;base64," + + Base64.getEncoder().encodeToString("".getBytes()))); + assertNull(InlineImageData.parse("data:text/html;base64,PHNjcmlwdD4=")); + assertNull(InlineImageData.parse(dataUrl("png", "GIF89a!".getBytes()))); + assertNull(InlineImageData.parse("data:image/png;base64,not base64!")); + assertNull(InlineImageData.parse("data:image/png," + PNG.substring(22))); + assertNull(InlineImageData.parse("https://example.org/a.png")); + assertNull(InlineImageData.parse(null)); + } + + @Test + void uploadParsingFollowsTheUploadLimitNotTheFieldLimit() { + byte[] big = new byte[100_000]; + System.arraycopy("GIF89a".getBytes(), 0, big, 0, 6); + String value = dataUrl("gif", big); + + assertNull(InlineImageData.parse(value)); + assertNotNull(InlineImageData.parseForUpload(value, 200_000)); + assertNull(InlineImageData.parseForUpload(value, 50_000)); + assertTrue(InlineImageData.exceedsUploadLimit(value, 50_000)); + } + + @Test + void findsDataSourcesWithTheirExactPositions() { + String html = "

a

\"x\"" + + ""; + List sources = InlineImageData.findSources(html); + + assertEquals(2, sources.size()); + assertEquals("src=\"" + PNG + "\"", + html.substring(sources.get(0).getStart(), sources.get(0).getEnd())); + assertEquals("SRC=" + PNG, + html.substring(sources.get(1).getStart(), sources.get(1).getEnd())); + assertEquals(PNG, sources.get(1).getValue()); + } + + @Test + void ignoresSrcTextOutsideTheSrcAttribute() { + String inAlt = "src=\"" + PNG + "\""; + assertTrue(InlineImageData.findSources(inAlt).isEmpty()); + + String dataSrc = ""; + assertTrue(InlineImageData.findSources(dataSrc).isEmpty()); + + String afterAlt = "src=x"; + List sources = InlineImageData.findSources(afterAlt); + assertEquals(1, sources.size()); + assertEquals(PNG, sources.get(0).getValue()); + } + + @Test + void findingSourcesStaysLinearOnAdversarialInput() { + String unclosedTags = ""; + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> { + assertTrue(InlineImageData.findSources(unclosedTags).isEmpty()); + assertTrue(InlineImageData.findSources(unclosedQuotes).isEmpty()); + }); + } + @Test void fieldLimitDefaultsWhenUnset() { assertEquals(InlineImageData.DEFAULT_MAX_FIELD_BYTES, withLimit(null)); From 6140216b964f690f484cc0144010ac4993f73345 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 08:22:40 -0400 Subject: [PATCH 4/9] Roll back failed entry edits before removing their uploaded images - Release the session when a save fails before the entry is stored, so the cleanup transaction cannot commit the failed edit. Look each upload up again by id before removing it. - Track an upload before creating it, so a record committed before a failed file write is still removed. - Scan img tags with a quote-aware linear scanner instead of a regex, so angle brackets in quoted attribute values do not hide a data source. --- .../ui/struts2/editor/EntryEdit.java | 17 +++- .../weblogger/util/InlineImageData.java | 93 ++++++++++++------- .../editor/EntryEditInlineImagesTest.java | 88 +++++++++++++++++- .../weblogger/util/InlineImageDataTest.java | 23 +++++ 4 files changed, 185 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java index f2739860a..da8eaab36 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java @@ -322,6 +322,10 @@ String save() { } catch (Exception e) { log.error("Error saving new entry", e); if (!entrySaved) { + // The entry may already hold the failed edit, and the + // cleanup below commits its own transaction. Roll the + // failed edit back first so that commit cannot save it. + WebloggerFactory.getWeblogger().release(); getBean().setText(submittedText); getBean().setSummary(submittedSummary); removeCreatedImages(WebloggerFactory.getWeblogger() @@ -482,12 +486,15 @@ private String replaceInlineImages(String html, media.setLength(image.getBytes().length); media.setContentType(image.getContentType()); media.setInputStream(new ByteArrayInputStream(image.getBytes())); + // Track the upload before creating it: createMediaFile commits + // the record before it writes the file, so a failed write can + // still leave a record to remove. + createdImages.add(media); mediaManager.createMediaFile(getActionWeblog(), media, errors); if (errors.getErrorCount() > 0) { addMediaErrors(errors); return html; } - createdImages.add(media); url = media.getPermalink(); mediaUrls.put(source.getValue(), url); } @@ -512,7 +519,13 @@ private void removeCreatedImages(MediaFileManager mediaManager, List createdImages) { for (MediaFile image : createdImages) { try { - mediaManager.removeMediaFile(getActionWeblog(), image); + // Look the upload up again: the save may have rolled back and + // released the session, and an attempted upload may never + // have been stored. + MediaFile stored = mediaManager.getMediaFile(image.getId()); + if (stored != null) { + mediaManager.removeMediaFile(stored.getWeblog(), stored); + } } catch (WebloggerException cleanupError) { log.warn("Could not remove an image from a failed entry save", cleanupError); } diff --git a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java index 7e98a191c..f3dfff03f 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/InlineImageData.java @@ -37,7 +37,6 @@ public final class InlineImageData { // MySQL's TEXT column holds 65,535 bytes. Leave room for the rest of an entry. static final int DEFAULT_MAX_FIELD_BYTES = 60000; - private static final Pattern IMAGE_TAG = Pattern.compile("(?is)]*>"); private static final Pattern DATA_URL = Pattern.compile( "(?i)^data:image/(png|jpeg|gif);base64,([a-z0-9+/]+={0,2})$"); @@ -68,70 +67,98 @@ public static List findSources(String html) { if (html == null) { return sources; } - Matcher image = IMAGE_TAG.matcher(html); - while (image.find()) { - Source source = findSource(image.group(), image.start()); - if (source != null - && source.value.trim().toLowerCase(Locale.ROOT).startsWith("data:")) { - sources.add(source); + int i = 0; + while ((i = indexOfImageTag(html, i)) >= 0) { + int[] tag = scanImageTag(html, i); + if (tag == null) { + // The tag never closes, so nothing after it is markup. + break; } + if (tag[1] >= 0) { + Source source = new Source(tag[1], tag[2], html.substring(tag[3], tag[4])); + if (source.value.trim().toLowerCase(Locale.ROOT).startsWith("data:")) { + sources.add(source); + } + } + i = tag[0]; } return sources; } + /** Index of the next "') { + result[0] = i + 1; + return result; + } if (Character.isWhitespace(c) || c == '/' || c == '=') { i++; continue; } - if (c == '>') { - return null; - } int nameStart = i; - while (i < length && !isNameEnd(tag.charAt(i))) { + while (i < length && !isNameEnd(html.charAt(i))) { i++; } - String name = tag.substring(nameStart, i); + boolean isSrc = "src".equalsIgnoreCase(html.substring(nameStart, i)); int afterName = i; - while (i < length && Character.isWhitespace(tag.charAt(i))) { + while (i < length && Character.isWhitespace(html.charAt(i))) { i++; } - if (i >= length || tag.charAt(i) != '=') { + if (i >= length || html.charAt(i) != '=') { i = afterName; continue; } i++; - while (i < length && Character.isWhitespace(tag.charAt(i))) { + while (i < length && Character.isWhitespace(html.charAt(i))) { i++; } - String value; - if (i < length && (tag.charAt(i) == '"' || tag.charAt(i) == '\'')) { - char quote = tag.charAt(i); - int close = tag.indexOf(quote, i + 1); + int valueStart; + int valueEnd; + if (i < length && (html.charAt(i) == '"' || html.charAt(i) == '\'')) { + int close = html.indexOf(html.charAt(i), i + 1); if (close < 0) { return null; } - value = tag.substring(i + 1, close); + valueStart = i + 1; + valueEnd = close; i = close + 1; } else { - int valueStart = i; - while (i < length && !Character.isWhitespace(tag.charAt(i)) - && tag.charAt(i) != '>') { + valueStart = i; + while (i < length && !Character.isWhitespace(html.charAt(i)) + && html.charAt(i) != '>') { i++; } - value = tag.substring(valueStart, i); + valueEnd = i; } - if ("src".equalsIgnoreCase(name)) { - return new Source(offset + nameStart, offset + i, value); + if (isSrc && result[1] < 0) { + result[1] = nameStart; + result[2] = i; + result[3] = valueStart; + result[4] = valueEnd; } } return null; diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java index 31bee5f64..5a50d853d 100644 --- a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java @@ -18,28 +18,38 @@ import java.util.ArrayList; import java.util.Base64; +import java.util.HashMap; import java.util.List; +import java.util.Locale; +import java.util.Map; +import org.apache.roller.weblogger.WebloggerException; import org.apache.roller.weblogger.business.FileContentManager; import org.apache.roller.weblogger.business.MediaFileManager; import org.apache.roller.weblogger.business.URLStrategy; import org.apache.roller.weblogger.business.Weblogger; import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.business.WeblogEntryManager; +import org.apache.roller.weblogger.business.UserManager; +import org.apache.roller.weblogger.business.search.IndexManager; import org.apache.roller.weblogger.config.WebloggerConfig; import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; import org.apache.roller.weblogger.pojos.MediaFile; import org.apache.roller.weblogger.pojos.MediaFileDirectory; import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.pojos.WeblogEntry; import org.apache.roller.weblogger.pojos.WeblogPermission; import org.apache.roller.weblogger.util.RollerMessages; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.InOrder; import org.mockito.MockedStatic; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -48,6 +58,8 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; @@ -65,6 +77,9 @@ class EntryEditInlineImagesTest { private MockedStatic runtimeConfig; private MockedStatic factory; private MediaFileManager mediaManager; + private Weblogger weblogger; + /** Media records the mocked manager has stored, by id. */ + private final Map stored = new HashMap<>(); private FileContentManager contentManager; private Weblog weblog; private EntryEdit action; @@ -83,7 +98,7 @@ void setUp() throws Exception { mediaManager = mock(MediaFileManager.class); contentManager = mock(FileContentManager.class); URLStrategy urls = mock(URLStrategy.class); - Weblogger weblogger = mock(Weblogger.class); + weblogger = mock(Weblogger.class); when(weblogger.getMediaFileManager()).thenReturn(mediaManager); when(weblogger.getFileContentManager()).thenReturn(contentManager); when(weblogger.getUrlStrategy()).thenReturn(urls); @@ -94,6 +109,13 @@ void setUp() throws Exception { .thenReturn(new MediaFileDirectory()); when(contentManager.canSave(any(), anyString(), anyString(), anyLong(), any())) .thenReturn(true); + doAnswer(call -> { + MediaFile media = call.getArgument(1); + stored.put(media.getId(), media); + return null; + }).when(mediaManager).createMediaFile(any(), any(), any()); + when(mediaManager.getMediaFile(anyString())) + .thenAnswer(call -> stored.get(call.getArgument(0))); weblog = mock(Weblog.class); when(weblog.hasUserPermission(any(), eq(WeblogPermission.POST))).thenReturn(true); @@ -212,4 +234,68 @@ void refusedUploadRestoresTheFieldsAndRemovesEarlierUploads() throws Exception { assertEquals(1, created.size()); verify(mediaManager).removeMediaFile(weblog, created.get(0)); } + + @Test + void uploadWhoseFileWriteFailsIsStillRemoved() throws Exception { + // createMediaFile commits the record, then fails writing the file + doAnswer(call -> { + MediaFile media = call.getArgument(1); + stored.put(media.getId(), media); + throw new WebloggerException("file write failed"); + }).when(mediaManager).createMediaFile(any(), any(), any()); + action.setEntry(new WeblogEntry()); + action.getBean().setText(""); + + assertEquals(EntryEdit.INPUT, action.save()); + + assertEquals(1, stored.size()); + MediaFile attempted = stored.values().iterator().next(); + verify(mediaManager).removeMediaFile(weblog, attempted); + assertEquals("", action.getBean().getText()); + } + + @Test + void attemptedUploadThatWasNeverStoredIsSkipped() throws Exception { + doAnswer(call -> { + throw new WebloggerException("refused before storing"); + }).when(mediaManager).createMediaFile(any(), any(), any()); + action.setEntry(new WeblogEntry()); + action.getBean().setText(""); + + assertEquals(EntryEdit.INPUT, action.save()); + + verify(mediaManager, never()).removeMediaFile(any(), any()); + } + + @Test + void failedEditIsRolledBackBeforeUploadsAreRemoved() throws Exception { + WeblogEntryManager entryManager = mock(WeblogEntryManager.class); + when(weblogger.getWeblogEntryManager()).thenReturn(entryManager); + when(weblogger.getIndexManager()).thenReturn(mock(IndexManager.class)); + when(weblogger.getUserManager()).thenReturn(mock(UserManager.class)); + // the category was deleted after the edit form was loaded + when(entryManager.getWeblogCategory("gone")).thenReturn(null); + doReturn(Locale.US).when(action).getLocale(); + WeblogEntry entry = new WeblogEntry(); + entry.setTitle("stored title"); + action.setEntry(entry); + action.getBean().setTitle("edited title"); + action.getBean().setStatus(WeblogEntry.PubStatus.DRAFT.name()); + action.getBean().setCategoryId("gone"); + action.getBean().setText(""); + + assertEquals(EntryEdit.INPUT, action.save()); + + // the edit reached the entry before failing... + assertEquals("edited title", entry.getTitle()); + MediaFile upload = stored.values().iterator().next(); + // ...so it is rolled back before the cleanup commits, and the + // entry is never saved + InOrder order = inOrder(weblogger, mediaManager); + order.verify(weblogger).release(); + order.verify(mediaManager).removeMediaFile(weblog, upload); + order.verify(weblogger).flush(); + verify(weblogger, times(1)).flush(); + verify(entryManager, never()).saveWeblogEntry(any()); + } } diff --git a/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java b/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java index 3b1deb245..fa6b303fc 100644 --- a/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/util/InlineImageDataTest.java @@ -109,13 +109,36 @@ void ignoresSrcTextOutsideTheSrcAttribute() { assertEquals(PNG, sources.get(0).getValue()); } + @Test + void findsSourcesInTagsWithAngleBracketsInQuotedValues() { + String before = "\"a b\" src=\"" + PNG + "\">"; + List sources = InlineImageData.findSources(before); + assertEquals(1, sources.size()); + assertEquals(PNG, sources.get(0).getValue()); + assertEquals("src=\"" + PNG + "\"", + before.substring(sources.get(0).getStart(), sources.get(0).getEnd())); + + String after = "

x

\"a"; + sources = InlineImageData.findSources(after); + assertEquals(2, sources.size()); + assertEquals(PNG, sources.get(0).getValue()); + assertEquals(PNG, sources.get(1).getValue()); + } + + @Test + void ignoresElementsThatOnlyStartWithImg() { + assertTrue(InlineImageData.findSources("").isEmpty()); + } + @Test void findingSourcesStaysLinearOnAdversarialInput() { String unclosedTags = ""; + String bracketsInQuotes = "\""".repeat(200_000) + "\">"; assertTimeoutPreemptively(Duration.ofSeconds(2), () -> { assertTrue(InlineImageData.findSources(unclosedTags).isEmpty()); assertTrue(InlineImageData.findSources(unclosedQuotes).isEmpty()); + assertTrue(InlineImageData.findSources(bracketsInQuotes).isEmpty()); }); } From 4aa233541b265fbc408e9b0ddb64cb40cf432b8d Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 13:09:42 -0400 Subject: [PATCH 5/9] Report an inline image over the field limit as too large With images kept inline, InlineImageData.parse() refuses a data URL longer than weblog.inlineImages.maxFieldBytes, and the caller reported every refusal as an unsupported or invalid image. Check the length first and report weblogEdit.inlineImageTooLarge. --- .../weblogger/ui/struts2/editor/EntryEdit.java | 6 ++++++ .../editor/EntryEditInlineImagesTest.java | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java index da8eaab36..3d2372cdb 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java @@ -417,6 +417,12 @@ private boolean validateInlineImages(List sources, addError("weblogEdit.inlineImageUploadTooLarge"); return false; } + // parse() also refuses an inline image over the field limit; report + // that as too large, not as an invalid image + if (keepInline && source.getValue().length() > InlineImageData.maxFieldBytes()) { + addError("weblogEdit.inlineImageTooLarge"); + return false; + } InlineImageData.Image image = keepInline ? InlineImageData.parse(source.getValue()) : InlineImageData.parseForUpload(source.getValue(), diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java index 5a50d853d..d405b8357 100644 --- a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditInlineImagesTest.java @@ -187,6 +187,23 @@ void refusesInlineFieldsOverTheLimit() throws Exception { assertEquals(text, action.getBean().getText()); } + @Test + void reportsAnInlineImageOverTheLimitAsTooLarge() throws Exception { + config.when(() -> WebloggerConfig.getBooleanProperty("weblog.inlineImages.preferInline")) + .thenReturn(true); + config.when(() -> WebloggerConfig.getProperty("weblog.inlineImages.maxFieldBytes")) + .thenReturn("100"); + String text = ""; + action.getBean().setText(text); + + assertFalse(action.prepareInlineImages(created)); + + assertTrue(action.getActionErrors().contains("weblogEdit.inlineImageTooLarge"), + action.getActionErrors().toString()); + assertFalse(action.getActionErrors().contains("weblogEdit.inlineImageInvalid")); + assertEquals(text, action.getBean().getText()); + } + @Test void refusesInvalidImageData() throws Exception { String text = " Date: Sun, 4 Oct 2026 13:23:13 -0400 Subject: [PATCH 6/9] Explain a form POST that is too large for the server A pasted image travels in the entry form as base64. When the form is over the container's maximum POST size (Tomcat: 2 MB by default), Tomcat drops every parameter, the salt check fails, and the author saw a 500 Security Violation. When Tomcat reports POST_TOO_LARGE, answer 413 with a message that says what happened, and document maxPostSize. --- CHANGES.md | 3 ++ .../ui/core/filters/ValidateSaltFilter.java | 25 ++++++++++++++ .../resources/ApplicationResources.properties | 1 + app/src/main/webapp/WEB-INF/web.xml | 5 +++ .../core/filters/ValidateSaltFilterTest.java | 34 +++++++++++++++++++ docs/roller-user-guide.adoc | 8 +++++ 6 files changed, 76 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index d963f8fa7..40691624c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,6 +15,9 @@ `MEDIUMTEXT` before you raise it. - Bundled themes allow `data:` images. Custom themes with their own Content Security Policy need `data:` in `img-src`. + - Pasted images travel in the form POST as base64. Tomcat's `maxPostSize` + defaults to 2 MB, so larger pastes are refused with a "form is too large" + message; raise `maxPostSize` to accept them. ## 6.1.6 diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilter.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilter.java index 78a32474b..c0aa87f34 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilter.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilter.java @@ -27,9 +27,11 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.util.I18nMessages; /** * Filter checks all POST request for presence of valid salt value and rejects those without @@ -38,6 +40,12 @@ public class ValidateSaltFilter implements Filter { private static final Log log = LogFactory.getLog(ValidateSaltFilter.class); + /** + * Request attribute Tomcat sets when it does not parse the parameters of a + * request; its value names the reason. + */ + static final String PARSE_FAILED_REASON = "org.apache.catalina.parameter_parse_failed_reason"; + @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { @@ -54,6 +62,18 @@ public void doFilter(ServletRequest request, ServletResponse response, try { SaltValidator.requireSubmittedSalt(httpReq); } catch (ServletException e) { + if (isPostTooLarge(httpReq)) { + // The container dropped the form, salt included, because it + // is over its maximum POST size. Pasted images make this + // likely, so say why instead of reporting a security error. + log.warn("Refused a POST to " + httpReq.getServletPath() + + " that is larger than the server's maximum POST size"); + ((HttpServletResponse) response).sendError( + HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, + I18nMessages.getMessages(httpReq.getLocale()) + .getString("error.postTooLarge")); + return; + } if (log.isDebugEnabled()) { log.debug("Valid salt value not found on POST to URL : " + httpReq.getServletPath()); @@ -73,6 +93,11 @@ public void init(FilterConfig filterConfig) throws ServletException { public void destroy() { } + static boolean isPostTooLarge(HttpServletRequest request) { + Object reason = request.getAttribute(PARSE_FAILED_REASON); + return reason != null && "POST_TOO_LARGE".equals(reason.toString()); + } + private boolean isStrutsAction(HttpServletRequest request) { String servletPath = request.getServletPath(); return servletPath != null && servletPath.endsWith(".rol"); diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index c10d10ff3..ba7a42b9c 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -1556,6 +1556,7 @@ weblogEdit.deleteEntry=Delete Entry weblogEdit.insertMediaFile=Insert Media File weblogEdit.inlineImageInvalid=This entry contains an unsupported or invalid embedded image. Use a PNG, JPEG or GIF image, or insert a media file. weblogEdit.inlineImageTooLarge=This entry has too much embedded image data. Resize the image or ask an administrator to enable media uploads. +error.postTooLarge=The form is too large to submit, so nothing was saved. If it contains pasted images, use smaller images or insert them as media files. An administrator can raise the server's maximum POST size. weblogEdit.inlineImageUploadTooLarge=This embedded image exceeds the site's media upload size limit. Resize it before saving. weblogEdit.fullPreviewMode=Full Preview weblogEdit.locale=Language diff --git a/app/src/main/webapp/WEB-INF/web.xml b/app/src/main/webapp/WEB-INF/web.xml index 8b1b550e3..865a69c66 100644 --- a/app/src/main/webapp/WEB-INF/web.xml +++ b/app/src/main/webapp/WEB-INF/web.xml @@ -486,6 +486,11 @@ /roller-ui/errors/error.jsp + + 413 + /roller-ui/errors/error.jsp + + 403 /roller-ui/errors/403.jsp diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilterTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilterTest.java index 5f0715bee..f312ffc0c 100644 --- a/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilterTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilterTest.java @@ -87,6 +87,40 @@ public void testDoFilterWithPostMethodAndInvalidSalt() throws Exception { } } + @Test + public void testPostTooLargeForTheContainerGetsA413() throws Exception { + try (MockedStatic mockedRollerSession = mockStatic(RollerSession.class)) { + mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession); + + when(request.getMethod()).thenReturn("POST"); + when(request.getServletPath()).thenReturn("/roller-ui/authoring/entryAdd.rol"); + when(request.getLocale()).thenReturn(java.util.Locale.ENGLISH); + // Tomcat dropped the parameters, so the salt is missing + when(request.getParameter("salt")).thenReturn(null); + when(request.getAttribute(ValidateSaltFilter.PARSE_FAILED_REASON)).thenReturn("POST_TOO_LARGE"); + + filter.doFilter(request, response, chain); + + verify(response).sendError(eq(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE), + argThat(message -> message.contains("too large"))); + verify(chain, never()).doFilter(request, response); + } + } + + @Test + public void testOtherParseFailuresStillFailTheSaltCheck() throws Exception { + try (MockedStatic mockedRollerSession = mockStatic(RollerSession.class)) { + mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession); + + when(request.getMethod()).thenReturn("POST"); + when(request.getParameter("salt")).thenReturn(null); + when(request.getAttribute(ValidateSaltFilter.PARSE_FAILED_REASON)).thenReturn("CLIENT_DISCONNECT"); + + assertThrows(ServletException.class, () -> filter.doFilter(request, response, chain)); + verify(response, never()).sendError(anyInt(), anyString()); + } + } + @Test public void testDoFilterWithPostMethodAndMismatchedUserId() throws Exception { try (MockedStatic mockedRollerSession = mockStatic(RollerSession.class); diff --git a/docs/roller-user-guide.adoc b/docs/roller-user-guide.adoc index 86401d5ea..07af8b845 100644 --- a/docs/roller-user-guide.adoc +++ b/docs/roller-user-guide.adoc @@ -415,6 +415,14 @@ limit with `weblog.inlineImages.maxFieldBytes`. On MySQL, alter the raising the limit, because a `TEXT` column holds only 65,535 bytes. Custom themes with a Content Security Policy must allow `data:` in `img-src` to show inline images. +A pasted image is sent with the entry form as base64 text, about 4/3 of the +image's size. Servlet containers limit the size of a form POST; Tomcat's +`maxPostSize` defaults to 2 MB, so a pasted image larger than about 1.5 MB is +refused with a "form is too large" message even when it is within the upload +limit. To accept larger pasted images, set `maxPostSize` on the Tomcat +`Connector` to at least 4/3 of `uploads.file.maxsize`, plus room for the +entry text. + If you’d like to upload images or other files for use in your weblog, go to your weblog’s *Create & Edit -> Media Files* page. From there you can upload files, browse and search files. You can also manage your files, From 03911f9003a3c406d34c8d6f4253655d36f971fb Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 13:24:44 -0400 Subject: [PATCH 7/9] Show a plain page for a form that is too large The generic error page called it an unexpected exception that had been logged. Use a page with a heading and the message. --- .../resources/ApplicationResources.properties | 1 + app/src/main/webapp/WEB-INF/web.xml | 2 +- app/src/main/webapp/roller-ui/errors/413.jsp | 35 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 app/src/main/webapp/roller-ui/errors/413.jsp diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index ba7a42b9c..9a2db49b1 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -461,6 +461,7 @@ error.password.mismatch=Wrong username and password combination error.unmatched.openid=Unknown or invalid OpenID URL error.title.403=Access Denied +error.title.413=Form too large error.text.403=You do not have the privileges necessary to access the requested page. error.title.404=Sorry! We couldn't find your document diff --git a/app/src/main/webapp/WEB-INF/web.xml b/app/src/main/webapp/WEB-INF/web.xml index 865a69c66..07e405a5e 100644 --- a/app/src/main/webapp/WEB-INF/web.xml +++ b/app/src/main/webapp/WEB-INF/web.xml @@ -488,7 +488,7 @@ 413 - /roller-ui/errors/error.jsp + /roller-ui/errors/413.jsp diff --git a/app/src/main/webapp/roller-ui/errors/413.jsp b/app/src/main/webapp/roller-ui/errors/413.jsp new file mode 100644 index 000000000..8c0d96609 --- /dev/null +++ b/app/src/main/webapp/roller-ui/errors/413.jsp @@ -0,0 +1,35 @@ +<%-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. The ASF licenses this file to You + under the Apache License, Version 2.0 (the "License"); you may not + use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. For additional information regarding + copyright in this work, please see the NOTICE file in the top level + directory of this distribution. +--%> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/fmt" prefix="fmt" %> + + + + + + + <fmt:message key="error.title.413" /> + + + +

+ +

+ + + From 7333029bcd5773ec497e7df7c37b3263dc06eff8 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 13:25:53 -0400 Subject: [PATCH 8/9] Tell the author how to get back to a form that was too large --- app/src/main/resources/ApplicationResources.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 9a2db49b1..7993cb78f 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -1557,7 +1557,7 @@ weblogEdit.deleteEntry=Delete Entry weblogEdit.insertMediaFile=Insert Media File weblogEdit.inlineImageInvalid=This entry contains an unsupported or invalid embedded image. Use a PNG, JPEG or GIF image, or insert a media file. weblogEdit.inlineImageTooLarge=This entry has too much embedded image data. Resize the image or ask an administrator to enable media uploads. -error.postTooLarge=The form is too large to submit, so nothing was saved. If it contains pasted images, use smaller images or insert them as media files. An administrator can raise the server's maximum POST size. +error.postTooLarge=The form is too large to submit, so nothing was saved. Use your browser's Back button to return to it. If it contains pasted images, use smaller images or insert them as media files. An administrator can raise the server's maximum POST size. weblogEdit.inlineImageUploadTooLarge=This embedded image exceeds the site's media upload size limit. Resize it before saving. weblogEdit.fullPreviewMode=Full Preview weblogEdit.locale=Language From c7a34f1144fd9b91910dda7b8d68c899ae824cae Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 4 Oct 2026 13:28:36 -0400 Subject: [PATCH 9/9] Keep the apostrophe in the inline image upload limit message Struts formats action errors with MessageFormat, which drops a single apostrophe, so the message read "the sites media upload size limit". --- app/src/main/resources/ApplicationResources.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 7993cb78f..5d6117f7b 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -1558,7 +1558,7 @@ weblogEdit.insertMediaFile=Insert Media File weblogEdit.inlineImageInvalid=This entry contains an unsupported or invalid embedded image. Use a PNG, JPEG or GIF image, or insert a media file. weblogEdit.inlineImageTooLarge=This entry has too much embedded image data. Resize the image or ask an administrator to enable media uploads. error.postTooLarge=The form is too large to submit, so nothing was saved. Use your browser's Back button to return to it. If it contains pasted images, use smaller images or insert them as media files. An administrator can raise the server's maximum POST size. -weblogEdit.inlineImageUploadTooLarge=This embedded image exceeds the site's media upload size limit. Resize it before saving. +weblogEdit.inlineImageUploadTooLarge=This embedded image exceeds the site''s media upload size limit. Resize it before saving. weblogEdit.fullPreviewMode=Full Preview weblogEdit.locale=Language weblogEdit.pendingEntries=Pending Entries