This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-codec.git
commit ab67d62f090393309e596773a2ab71b4d58762d3 Author: Gary Gregory <[email protected]> AuthorDate: Sun Sep 20 16:33:20 2026 -0400 [CODEC-345] URLCodec.encodeUrl(BitSet, byte[]) ignores the caller-supplied BitSet for % and + --- src/changes/changes.xml | 1 + .../org/apache/commons/codec/net/URLCodec.java | 52 ++++++++++++++++---- .../org/apache/commons/codec/net/Codec345Test.java | 56 ++++++++++++++++++++++ .../org/apache/commons/codec/net/URLCodecTest.java | 17 ++++--- 4 files changed, 109 insertions(+), 17 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d02fd1e6..8afdb972 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -68,6 +68,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Bound Sha2Crypt rounds from caller-supplied salts.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Bound Sha2Crypt password length to prevent quadratic CPU DoS.</action> <action type="fix" dev="ggregory" due-to="geonseok, Gary Gregory">Make the @param names in BaseNCodec match the parameters they document (#444).</action> + <action type="fix" dev="ggregory" due-to="Ganesh Gautam, Gary Gregory" issue="CODEC-345">Restore URLCodec.encodeUrl(BitSet, byte[]) support for custom safe sets containing percent and plus, and document the decoding limitations.</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use PhoneticEngine.Builder and deprecate old constructors.</action> <action type="add" dev="ggregory" due-to="Gary Gregory">Add BeiderMorseEncoder.Builder and deprecate old constructor.</action> diff --git a/src/main/java/org/apache/commons/codec/net/URLCodec.java b/src/main/java/org/apache/commons/codec/net/URLCodec.java index cc28869b..f48bdb95 100644 --- a/src/main/java/org/apache/commons/codec/net/URLCodec.java +++ b/src/main/java/org/apache/commons/codec/net/URLCodec.java @@ -98,6 +98,12 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St * Decodes an array of URL safe 7-bit characters into an array of original bytes. Escaped characters are converted * back to their original representation. * + * <p> + * Decoding always follows {@code www-form-urlencoded} rules: {@code +} becomes a space and {@code %} starts a hexadecimal escape. + * Output from {@link #encodeUrl(BitSet, byte[])} with a custom safe set may therefore not decode back to the original input and may cause a + * {@link DecoderException}, depending on which characters were marked safe. + * </p> + * * @param bytes * array of URL safe characters. * @return array of original bytes. @@ -129,14 +135,20 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St } /** - * Encodes an array of bytes into an array of URL safe 7-bit characters. Unsafe characters are escaped. - * The characters {@code %} and {@code +} are always escaped because {@link #decodeUrl(byte[])} - * treats them as URL-encoding syntax. + * Encodes an array of bytes using the given set of URL safe characters. + * <p> + * Unsafe characters are percent-escaped. Characters marked safe are copied unchanged, except that a space marked safe is converted to {@code +}. A + * {@code null} bitset selects the default {@code www-form-urlencoded} safe set, which escapes both {@code %} and {@code +}. + * </p> + * <p> + * A custom bitset can produce output that {@link #decodeUrl(byte[])} and the {@code decode} methods cannot decode back to the original input. These + * decoders always convert {@code +} to a space and interpret {@code %} as the start of a hexadecimal escape, regardless of the bitset used for encoding. If + * the custom bitset marks either character safe, decoding can change the original data or throw {@link DecoderException}. Callers using a custom bitset + * must choose decoding rules appropriate to that bitset and the URI component being encoded. + * </p> * - * @param urlsafe - * bitset of characters deemed URL safe, except for {@code %} and {@code +}. - * @param bytes - * array of bytes to convert to URL safe characters. + * @param urlsafe bitset of characters deemed URL safe, or {@code null} to use the default {@code www-form-urlencoded} safe set. + * @param bytes array of bytes to convert to URL safe characters. * @return array of bytes containing URL safe characters. */ public static final byte[] encodeUrl(BitSet urlsafe, final byte[] bytes) { @@ -153,7 +165,7 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St if (b < 0) { b = 256 + b; } - if (urlsafe.get(b) && b != ESCAPE_CHAR && b != PLUS_CHAR) { + if (urlsafe.get(b)) { if (b == ' ') { b = PLUS_CHAR; } @@ -197,6 +209,12 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St * Decodes an array of URL safe 7-bit characters into an array of original bytes. Escaped characters are converted * back to their original representation. * + * <p> + * Decoding always follows {@code www-form-urlencoded} rules: {@code +} becomes a space and {@code %} starts a hexadecimal escape. + * Output from {@link #encodeUrl(BitSet, byte[])} with a custom safe set may therefore not decode back to the original input and may cause a + * {@link DecoderException}, depending on which characters were marked safe. + * </p> + * * @param bytes * array of URL safe characters. * @return array of original bytes. @@ -212,6 +230,12 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St * Decodes a URL safe object into its original form. Escaped characters are converted back to their original * representation. * + * <p> + * Decoding always follows {@code www-form-urlencoded} rules: {@code +} becomes a space and {@code %} starts a hexadecimal escape. + * Output from {@link #encodeUrl(BitSet, byte[])} with a custom safe set may therefore not decode back to the original input and may cause a + * {@link DecoderException}, depending on which characters were marked safe. + * </p> + * * @param obj * URL safe object to convert into its original form. * @return original object. @@ -237,6 +261,12 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St * Decodes a URL safe string into its original form using the default string charset. Escaped characters are * converted back to their original representation. * + * <p> + * Decoding always follows {@code www-form-urlencoded} rules: {@code +} becomes a space and {@code %} starts a hexadecimal escape. + * Output from {@link #encodeUrl(BitSet, byte[])} with a custom safe set may therefore not decode back to the original input and may cause a + * {@link DecoderException}, depending on which characters were marked safe. + * </p> + * * @param str * URL safe string to convert into its original form. * @return original string. @@ -260,6 +290,12 @@ public class URLCodec implements BinaryEncoder, BinaryDecoder, StringEncoder, St * Decodes a URL safe string into its original form using the specified encoding. Escaped characters are converted * back to their original representation. * + * <p> + * Decoding always follows {@code www-form-urlencoded} rules: {@code +} becomes a space and {@code %} starts a hexadecimal escape. + * Output from {@link #encodeUrl(BitSet, byte[])} with a custom safe set may therefore not decode back to the original input and may cause a + * {@link DecoderException}, depending on which characters were marked safe. + * </p> + * * @param str * URL safe string to convert into its original form. * @param charsetName diff --git a/src/test/java/org/apache/commons/codec/net/Codec345Test.java b/src/test/java/org/apache/commons/codec/net/Codec345Test.java new file mode 100644 index 00000000..861ff1be --- /dev/null +++ b/src/test/java/org/apache/commons/codec/net/Codec345Test.java @@ -0,0 +1,56 @@ +/* + * 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 + * + * https://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.commons.codec.net; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.nio.charset.StandardCharsets; +import java.util.BitSet; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Tests the reproduction in <a href="https://issues.apache.org/jira/browse/CODEC-345";>CODEC-345</a>. + */ +class Codec345Test { + + @ParameterizedTest + @ValueSource(strings = { "/pages/1/Test+Page", "/display/TST/Caf%C3%A9" }) + void testEncodeUrlWithCallerSuppliedSafeCharacters(final String input) { + // RFC 2396 abs_path, as used by HtmlUnit's UrlUtils and HttpClient 3.x's URI. + final BitSet allowed = new BitSet(256); + for (int c = 'a'; c <= 'z'; c++) { + allowed.set(c); + } + for (int c = 'A'; c <= 'Z'; c++) { + allowed.set(c); + } + for (int c = '0'; c <= '9'; c++) { + allowed.set(c); + } + for (final char c : "-_.!~*'()".toCharArray()) { + allowed.set(c); // mark + } + allowed.set('%'); // escaped + for (final char c : ":@&=+$,;/".toCharArray()) { + allowed.set(c); // pchar + } + assertEquals(input, new String(URLCodec.encodeUrl(allowed, input.getBytes(StandardCharsets.UTF_8)), StandardCharsets.US_ASCII)); + } +} diff --git a/src/test/java/org/apache/commons/codec/net/URLCodecTest.java b/src/test/java/org/apache/commons/codec/net/URLCodecTest.java index fa374e31..a0c8b421 100644 --- a/src/test/java/org/apache/commons/codec/net/URLCodecTest.java +++ b/src/test/java/org/apache/commons/codec/net/URLCodecTest.java @@ -174,35 +174,34 @@ class URLCodecTest { @Test void testEncodeUrlWithNullBitSet() throws Exception { final URLCodec urlCodec = new URLCodec(); - final String plain = "Hello there!"; + final String plain = "Hello there!%+"; final String encoded = new String(URLCodec.encodeUrl(null, plain.getBytes(StandardCharsets.UTF_8))); - assertEquals("Hello+there%21", encoded, "Basic URL encoding test"); + assertEquals("Hello+there%21%25%2B", encoded, "Basic URL encoding test"); assertEquals(plain, urlCodec.decode(encoded), "Basic URL decoding test"); validateState(urlCodec); } @Test - void testEncodeUrlWithPercentMarkedSafeEscapesPercent() throws Exception { + void testEncodeUrlWithPercentMarkedSafePreservesPercent() { final BitSet safe = new BitSet(); safe.set('%'); final String plain = "%"; final byte[] encoded = URLCodec.encodeUrl(safe, plain.getBytes(StandardCharsets.US_ASCII)); final String encodedS = new String(encoded, StandardCharsets.US_ASCII); - assertEquals("%25", encodedS, "URLCodec should escape percent even when marked safe"); - final byte[] decoded = URLCodec.decodeUrl(encoded); - assertEquals(plain, new String(decoded, StandardCharsets.US_ASCII), "URLCodec percent decoding test"); + assertEquals(plain, encodedS, "URLCodec should preserve percent when marked safe"); + assertThrows(DecoderException.class, () -> URLCodec.decodeUrl(encoded)); } @Test - void testEncodeUrlWithPlusMarkedSafeEscapesPlus() throws Exception { + void testEncodeUrlWithPlusMarkedSafePreservesPlus() throws Exception { final BitSet safe = new BitSet(); safe.set('+'); final String plain = "+"; final byte[] encoded = URLCodec.encodeUrl(safe, plain.getBytes(StandardCharsets.US_ASCII)); final String encodedS = new String(encoded, StandardCharsets.US_ASCII); - assertEquals("%2B", encodedS, "URLCodec should escape plus even when marked safe"); + assertEquals(plain, encodedS, "URLCodec should preserve plus when marked safe"); final byte[] decoded = URLCodec.decodeUrl(encoded); - assertEquals(plain, new String(decoded, StandardCharsets.US_ASCII), "URLCodec plus decoding test"); + assertEquals(" ", new String(decoded, StandardCharsets.US_ASCII), "Default decoding interprets a literal plus as a space"); } @Test
