Added properties for base64 formatting by kuzjka · Pull Request #504 · apache/santuario-xml-security-java

kuzjka · 2025-08-11T17:21:13Z

New feature

This PR continues the mailing list thread and suggests system properties to control base64Binary values formatting:

org.apache.xml.security.base64.lineSeparator, org.apache.xml.security.base64.lineLength - allows to override default MIME encoding settings
org.apache.xml.security.base64.ignoreLineBreaks - disables line wrapping for base64 value, but allows to keep the whole XML pretty-printed. Takes precedence over line wrapping settings above.
existing org.apache.xml.security.ignoreLineBreaks takes precedence over all new options.

Default values result to CRLF line breaks with 76 chars in the line, making the whole thing fully compatible with previous implementation.

Other changes

Some of the logic is included to XMLUtils to provide better encapsulation:

adding line breaks in the beginning and in the end of wrapped base64 values in the element is implemented as XMLUtils.encodeElementValue
Base64OutputStream from Commons, used in XML encryption, is replaced with a java.util implementation and provided by XMLUtils, making it consistent with configured Base64 encoder.

Base64.Encoder/Decoder instances are thread-safe, so sharing a single instance should give us a little performance gain.

Test coverage

As the base64 configuration takes place during class load, unit-tests leverage from custom ClassLoader and Reflection API to reinitialize the class in each test.

Motivation

https://www.w3.org/TR/xmlschema-2/#base64Binary - base64Binary definition has note, that original RFC2045 line-length limitation must not be enforced. In current implementation it is only possible to remove all line breaks in the document using org.apache.xml.security.ignoreLineBreaks, sacrificing human readability.

Also, using LF instead of CRLF may be desired in systems where verifying side does not expect escape sequences in base64Binary values.

I would be thankful for your comments and suggestions.

seanjmullan · 2025-10-31T21:01:20Z

How about also removing org.apache.xml.security.utils.Base64? Is it needed anymore?

kuzjka · 2025-10-31T21:42:51Z

I can't see any usage of this class inside Santuario. However, I didn't touch it to avoid breaking changes. Despite deprecated, I can see it was retained in 8016fc3 for compatibility. Probably, @coheigea would give us better insight.

seanjmullan

Some comments on a first pass.

seanjmullan · 2025-10-31T20:48:46Z

src/main/java/org/apache/xml/security/utils/XMLUtils.java

 * DOM and XML accessibility and comfort functions.
 *
+ * @implNote
+ * Following system properties affect XML formatting:


Nit: "The following ..."

fixed in 8a598d0, thank you

seanjmullan · 2025-11-04T14:17:46Z

.../java/org/apache/xml/security/stax/impl/processor/output/AbstractEncryptOutputProcessor.java

-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
+import java.util.*;


Can you revert this change? I think it is better not to wildcard imports.

Oops, my IDE did it :) reverted

seanjmullan · 2025-11-04T14:49:53Z

src/main/java/org/apache/xml/security/utils/XMLUtils.java

+            return ignoreLineBreaks;
+        }
+
+        public void setIgnoreLineBreaks(boolean ignoreLineBreaks) {


Instead of set methods, why not add a constructor that takes the options and also checks for illegal syntax?

Since we would like to retain meaningful log messages for invalid property values, I couldn't find a better way than to move all properties reading together with validation logic to the constructor of Base64FormattingOptions.

Added warnings in case a property is set, but ignored.
Added handling of NumberFormatException as suggested in the comment below.

Example log:

testIgnoreLineBreaksTakesPrecedence: ... org.apache.xml.security.ignoreLineBreaks property takes precedence over org.apache.xml.security.base64.ignoreLineBreaks, line breaks will be ignored ... Property org.apache.xml.security.base64.lineSeparator has no effect since line breaks are ignored ... Property org.apache.xml.security.base64.lineLength has no effect since line breaks are ignored testIllegalPropertiesAreIgnored: ... Illegal value of org.apache.xml.security.base64.lineSeparator property is ignored: illegal ... Illegal value of org.apache.xml.security.base64.lineLength property is ignored: illegal

seanjmullan · 2025-11-04T14:57:45Z

src/main/java/org/apache/xml/security/utils/XMLUtils.java

-                    (PrivilegedAction<Boolean>) () -> Boolean.getBoolean("org.apache.xml.security.ignoreLineBreaks"));
+                    (PrivilegedAction<Boolean>) () -> Boolean.getBoolean(IGNORE_LINE_BREAKS_PROP));
+
+    private static Base64FormattingOptions base64Formatting =


if org.apache.xml.security.ignoreLineBreaks is true, none of these options matter, so did you consider skipping the getting of the other properties?

Also should log a warning if the other properties are set, since they have no impact if org.apache.xml.security.ignoreLineBreaks is true.

Were these comments addressed?

The warnings were added in c8f4e25.

In my opinion, it would be good to read the properties anyway for the sake of consistency. In the end, we still need to check them to produce warnings.

seanjmullan · 2025-11-04T16:06:30Z

src/main/java/org/apache/xml/security/utils/XMLUtils.java

+                }

-    private static final Logger LOG = System.getLogger(XMLUtils.class.getName());
+                Integer lineLength = Integer.getInteger(BASE64_LINE_LENGTH_PROP);


I think it would be better to use System.getProperty and then Integer.valueOf(String) because Integer.getInteger does not distinguish between the property not being set and an invalid value for the integer.

seanjmullan · 2025-11-07T13:53:46Z

src/main/java/org/apache/xml/security/utils/XMLUtils.java

+        /**
+         * Creates new formatting options by reading system properties.
+         */
+        public Base64FormattingOptions() {


The constructors and methods of this class should be package-private since the class is package-private.

seanjmullan · 2025-11-07T13:55:12Z

src/main/java/org/apache/xml/security/utils/XMLUtils.java

+            this.bytes = bytes;
+        }
+
+        public byte[] getBytes() {


Change to package-private.

src/test/java/org/apache/xml/security/utils/XMLUtilsTest.java

seanjmullan · 2025-11-07T14:01:32Z

src/test/java/org/apache/xml/security/utils/XMLUtilsTest.java

+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
+import static org.junit.jupiter.api.Assertions.*;
+


Add a comment describing what this test does.

src/main/java/org/apache/xml/security/utils/XMLUtils.java

coheigea · 2025-11-11T10:02:25Z

8016fc3

I think we are good to remove Base64, as OpenSAML stopped using it here https://shibboleth.atlassian.net/browse/OSJ-287

kuzjka · 2025-11-11T17:51:08Z

I think we are good to remove Base64

Thank you for clearing it up. There are also two duplicating getters XMLUtils#ignoreLineBreaks() and XMLUtils#isIgnoreLineBreaks(). After removing Base64 neither of them is used internally. Probably we can deprecate at least one of them.

Please let me know if you would like me to do this cleanup as a separate change.

coheigea · 2025-11-12T12:22:59Z

If they're not used then let's deprecated both of them, thanks.

seanjmullan · 2025-11-13T21:33:52Z

I think we are good to remove Base64

Thank you for clearing it up. There are also two duplicating getters XMLUtils#ignoreLineBreaks() and XMLUtils#isIgnoreLineBreaks(). After removing Base64 neither of them is used internally. Probably we can deprecate at least one of them.

Please let me know if you would like me to do this cleanup as a separate change.

Probably best to do this as a separate change just in case we have to later restore them for some reason.

.../java/org/apache/xml/security/stax/impl/processor/output/AbstractEncryptOutputProcessor.java

seanjmullan · 2025-12-05T20:57:10Z

src/test/java/org/apache/xml/security/utils/XMLUtilsTest.java

+ * Output of the first two methods is checked using an appropriate {@link FormattingChecker} implementation.
+ * The result of stream encoding is compared to the output of {@code encodeToString} method.
+ *
+ * There are also tests, which check that the corresponding decoding methods can process Base64-encoded data with any


Nit, no comma needed.

seanjmullan · 2025-12-05T21:00:20Z

src/test/java/org/apache/xml/security/utils/XMLUtilsTest.java

+
+    @Test
+    public void testEncodeToString() {
+        byte[] data =  new byte[60]; // long enough for a line break in MIME encoding


nit, extra space after '='. Same comment on other lines.

src/main/java/org/apache/xml/security/utils/XMLUtils.java

kuzjka · 2025-12-06T14:43:23Z

Sorry for the delay, this new approach looks good.

No problem, I had some tough weeks, so couldn't finish it earlier anyway.

Please check if StAX-based API is used correctly. I'm not quite sure how to verify the signature value and get decrypted data, but perhaps we don't need to do the verification, as the focus of the test is just to check that different formatting variants are consumed correctly.

seanjmullan · 2026-01-05T14:46:23Z

Sorry for the delay, this new approach looks good.

No problem, I had some tough weeks, so couldn't finish it earlier anyway.

Please check if StAX-based API is used correctly. I'm not quite sure how to verify the signature value and get decrypted data, but perhaps we don't need to do the verification, as the focus of the test is just to check that different formatting variants are consumed correctly.

I'm not familiar with the StAX-based API, so maybe @coheigea can check.

coheigea · 2026-01-26T15:50:46Z

@kuzjka Can you rebase the branch off the latest code please? There's a conflict.

…e test

* added FormattingTest annotation (JUnit tagging) * added FormattingChecker interface, various implementations for different formatting configurations and a factory to get appropriate implementation * added formatting options properties sets and multiple executions for Surefire plugin * refactored XMLUtilsTest: made it a @FormattingTest, removed hacks with classloader

kuzjka · 2026-01-27T00:35:03Z

@kuzjka Can you rebase the branch off the latest code please? There's a conflict.

Done.
The conflict was related to the changes in Apache Commons API for stream decorator in AbstractEncryptOutputProcessor, and this change suggests to make XMLUtils responsible for stream encoding (relying on JDK built-in Base64 utilities).

coheigea · 2026-02-04T13:53:35Z

Note the PMD violations in the build:

Error:  Failed to execute goal org.apache.maven.plugins:maven-pmd-plugin:3.21.0:check (pmd-verify) on project xmlsec: You have 22 PMD violations. For more details see: /home/runner/work/santuario-xml-security-java/santuario-xml-security-java/target/pmd.xml -> [Help 1]

seanjmullan reviewed Nov 4, 2025

View reviewed changes

seanjmullan reviewed Nov 7, 2025

View reviewed changes

src/main/java/org/apache/xml/security/utils/XMLUtils.java Show resolved Hide resolved

seanjmullan reviewed Dec 5, 2025

View reviewed changes

.../java/org/apache/xml/security/stax/impl/processor/output/AbstractEncryptOutputProcessor.java Show resolved Hide resolved

seanjmullan reviewed Dec 5, 2025

View reviewed changes

kuzjka force-pushed the dev/base64_formatting branch from 969eb8d to 5917ca9 Compare January 26, 2026 22:57

kuzjka and others added 6 commits January 27, 2026 01:18

Added properties for base64 formatting

55e1b1d

updated XMLUtils in response to review comments

c8f4e25

review: fixed access modifiers, added license and more comments in th…

92df1bc

…e test

added high-level formatting tests for signature and encryption

45340f0

fixed grammar and codestyle in XMLUtilsTest.java

4e2b189

kuzjka force-pushed the dev/base64_formatting branch from 5917ca9 to 4e2b189 Compare January 27, 2026 00:25

changed test class/method modifiers to pkg-private (PMD rule)

d8007ab

Conversation

kuzjka commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New feature

Other changes

Test coverage

Motivation

Uh oh!

seanjmullan commented Oct 31, 2025

Uh oh!

kuzjka commented Oct 31, 2025

Uh oh!

seanjmullan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coheigea commented Nov 11, 2025

Uh oh!

kuzjka commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coheigea commented Nov 12, 2025

Uh oh!

seanjmullan commented Nov 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kuzjka commented Dec 6, 2025

Uh oh!

seanjmullan commented Jan 5, 2026

Uh oh!

coheigea commented Jan 26, 2026

Uh oh!

kuzjka commented Jan 27, 2026

Uh oh!

coheigea commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kuzjka commented Aug 11, 2025 •

edited

Loading

kuzjka commented Nov 11, 2025 •

edited

Loading