Revert "Issue #13683 - UrlParameterDecoder fixed for mixed bytes/chars pct-encoding usage."

This reverts commit b8e8d37.

Author: joakime

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/CharsetStringBuilder.java

@@ -22,10 +22,13 @@
 import java.util.Arrays;
 import java.util.Objects;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * <p>Build a string from a sequence of bytes and/or characters.</p>
  * <p>Implementations of this interface are optimized for processing a mix of calls to already decoded
- * character based appends (e.g. {@link #append(char)}) and calls to undecoded byte methods (e.g. {@link #append(char)}).
+ * character based appends (e.g. {@link #append(char)} and calls to undecoded byte methods (e.g. {@link #append(byte)}.
  * This is particularly useful for decoding % encoded strings that are mostly already decoded but may contain
  * escaped byte sequences that are not decoded.  The standard {@link CharsetDecoder} API is not well suited for this
  * use-case.</p>
@@ -271,6 +274,7 @@ public void reset()
 
     class DecoderStringBuilder implements CharsetStringBuilder
     {
+        private static final Logger LOG = LoggerFactory.getLogger(DecoderStringBuilder.class);
         private final CharsetDecoder _decoder;
         private final StringBuilder _stringBuilder = new StringBuilder(32);
         private ByteBuffer _buffer = ByteBuffer.allocate(32);
@@ -304,17 +308,6 @@ public void append(char c)
         {
             if (_buffer.position() > 0)
             {
-                // We have a (possible) sequence started, need to continue it.
-                if (c > 0xFF)
-                {
-                    ensureSpace(2);
-                    _buffer.putChar(c);
-                }
-                else
-                {
-                    ensureSpace(1);
-                    _buffer.put((byte)c);
-                }
                 try
                 {
                     // Append any data already in the decoder
@@ -328,19 +321,28 @@ public void append(char c)
                     throw new RuntimeException(e);
                 }
             }
-            else
-            {
-                _stringBuilder.append(c);
-            }
+            _stringBuilder.append(c);
         }
 
         @Override
         public void append(CharSequence chars, int offset, int length)
         {
-            for (int idx = offset; idx < offset + length; idx++)
+            if (_buffer.position() > 0)
             {
-                append(chars.charAt(idx));
+                try
+                {
+                    // Append any data already in the decoder
+                    _stringBuilder.append(_decoder.decode(_buffer.flip()));
+                    _buffer.clear();
+                }
+                catch (CharacterCodingException e)
+                {
+                    // This will be thrown only if the decoder is configured to REPORT,
+                    // otherwise errors will be ignored or replaced and we will not catch here.
+                    throw new RuntimeException(e);
+                }
             }
+            _stringBuilder.append(chars, offset, offset + length);
         }
 
         @Override

jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URLEncodedTest.java

@@ -14,7 +14,6 @@
 package org.eclipse.jetty.util;
 
 import java.io.ByteArrayInputStream;
-import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -34,7 +33,6 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.DynamicTest.dynamicTest;
@@ -304,33 +302,4 @@ public void testInvalidDecodeTo(String inputString, Charset charset, Class<? ext
             UrlEncoded.decodeTo(inputString, map, charset);
         });
     }
-
-    /**
-     * Test of non-standard encoding of Shift_JIS.
-     * This tests a 2 byte sequence making up 1 Shift_JIS character.
-     * But only the first byte is pct-encoded, other byte is "in the raw" and not encoded.
-     * Both bytes are required for the KATAKANA LETTER HO: ホ to be decoded.
-     * See https://unicodeplus.com/U+30DB
-     */
-    @Test
-    public void testSjisNonStandardForm() throws IOException
-    {
-        // Actual x-www-form-urlencoded sent from Google Chrome 1.141 with Shift-JIS encoding.
-        // The same form data is sent from Firefox 143.0.4
-        // The same form data is also what is sent from Apache HttpClient 4.5.x and 5.x
-        // The properly encoded form of this character would be "a=%83%7A"
-        // Note: "%7A" is the ASCII "z"
-        String sjis = "a=%83z";
-        Charset shiftJis = Charset.forName("Shift_JIS");
-        byte[] bytes = sjis.getBytes(shiftJis);
-        try (ByteArrayInputStream in = new ByteArrayInputStream(bytes))
-        {
-            MultiMap<String> map = new MultiMap<>();
-            UrlEncoded.decodeTo(in, map, shiftJis, 500, 500);
-            List<String> result = map.get("a");
-            assertNotNull(result);
-            assertEquals(1, result.size());
-            assertEquals("ホ", result.get(0));
-        }
-    }
 }

jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/UrlParameterDecoderTest.java

@@ -31,7 +31,6 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
-import org.junit.jupiter.params.provider.ValueSource;
 
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static java.nio.charset.StandardCharsets.US_ASCII;
@@ -260,16 +259,13 @@ public void testBadlyEncodedValue() throws IOException
         assertEquals("Euro-€-Symbol", field.getValue(), "Fields[Name]");
     }
 
-    @ParameterizedTest
-    @ValueSource(strings = {
-        "name\n=value+%00%30&name1=&name2&nãme3=value+3", // without BOM
-        "name\n=value+%FE%FF%00%30&name1=&name2&nãme3=value+3" // with BOM
-    })
-    public void testUtf16EncodedString(String input) throws IOException
+    @Test
+    public void testUtf16EncodedString() throws IOException
     {
         Fields fields = new Fields();
         CharsetStringBuilder charsetStringBuilder = CharsetStringBuilder.forCharset(UTF_16);
         UrlParameterDecoder decoder = new UrlParameterDecoder(charsetStringBuilder, fields::add);
+        String input = "name\n=value+%FE%FF%00%30&name1=&name2&nãme3=value+3";
         assertFalse(decoder.parse(input), "No coding errors");
 
         assertThat("Field count", fields.getSize(), is(4));