[java-support] 14/21: Redo handling of unsafe data, and fix tests.
Scott Cantor
cantor.2 at osu.edu
Thu Jun 2 14:39:01 UTC 2022
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch dev/JSPT-111
in repository java-support.
View the commit online:
http://git.shibboleth.net/view/?p=java-support.git;a=commit;h=a6cde60a59003515c703a09ff241f0945b52b435
commit a6cde60a59003515c703a09ff241f0945b52b435
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Sep 20 10:28:11 2021 -0400
Redo handling of unsafe data, and fix tests.
---
.../shibboleth/utilities/java/support/ddf/DDF.java | 54 ++++++++++++++++----
.../support/ddf/RemotedHttpServletRequest.java | 59 +++++++++++++++++++---
.../support/ddf/RemotedHttpServletResponse.java | 31 ++++++++----
.../utilities/java/support/ddf/DDFTest.java | 12 ++---
.../support/ddf/RemotedHttpServletRequestTest.java | 14 ++---
.../ddf/RemotedHttpServletResponseTest.java | 12 ++---
6 files changed, 137 insertions(+), 45 deletions(-)
diff --git a/src/main/java/net/shibboleth/utilities/java/support/ddf/DDF.java b/src/main/java/net/shibboleth/utilities/java/support/ddf/DDF.java
index 14a29c7..3689c7f 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/ddf/DDF.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/ddf/DDF.java
@@ -208,6 +208,19 @@ public class DDF implements Iterable<DDF> {
string(val);
}
+ /**
+ * Constructor.
+ *
+ * <p>For compatibility, the name is constrained to <= 255 characters.</p>
+ *
+ * @param n node name
+ * @param val byte array value, handled without knowledge of the encoding
+ */
+ public DDF(@Nullable @NotEmpty final String n, @Nullable final byte[] val) {
+ this(n);
+ unsafe_string(val);
+ }
+
/**
* Constructor.
*
@@ -284,7 +297,7 @@ public class DDF implements Iterable<DDF> {
break;
case DDF_STRING_UNSAFE:
- dup.unsafe_string((String) value);
+ dup.unsafe_string((byte[]) value);
break;
case DDF_INT:
@@ -366,12 +379,21 @@ public class DDF implements Iterable<DDF> {
}
/**
- * Returns true iff the node is a string (safe or not).
+ * Returns true iff the node is a string.
*
- * @return true iff the node is a string (safe or not)
+ * @return true iff the node is a string
*/
public boolean isstring() {
- return type == DDFType.DDF_STRING || type == DDFType.DDF_STRING_UNSAFE;
+ return type == DDFType.DDF_STRING;
+ }
+
+ /**
+ * Returns true iff the node is an unsafe string.
+ *
+ * @return true iff the node is an unsafe string
+ */
+ public boolean isunsafestring() {
+ return type == DDFType.DDF_STRING_UNSAFE;
}
/**
@@ -430,6 +452,17 @@ public class DDF implements Iterable<DDF> {
return isstring() ? (String) value : null;
}
+ /**
+ * Get the byte array value of this node if an unsafe string.
+ *
+ * @return the byte array value or null
+ */
+// Checkstyle: MethodName OFF
+ @Nullable public byte[] unsafe_string() {
+ return isunsafestring() ? (byte[]) value : null;
+ }
+// Checkstyle: MethodName ON
+
/**
* Get the integer value of this node.
*
@@ -542,7 +575,7 @@ public class DDF implements Iterable<DDF> {
* @return this object
*/
// Checkstyle: MethodName OFF
- @Nonnull public DDF unsafe_string(@Nullable final String val) {
+ @Nonnull public DDF unsafe_string(@Nullable final byte[] val) {
empty();
value = val;
type = DDFType.DDF_STRING_UNSAFE;
@@ -1032,15 +1065,15 @@ public class DDF implements Iterable<DDF> {
break;
case DDF_STRING_UNSAFE:
- builder.append("char[]");
+ builder.append("byte[]");
if (name != null) {
builder.append(' ').append(name);
}
builder.append(" = ");
if (value != null) {
builder.append('{');
- for (final char c : ((String) value).toCharArray()) {
- builder.append(Integer.toHexString(c)).append(", ");
+ for (final byte b : (byte[]) value) {
+ builder.append(Integer.toHexString(b)).append(", ");
}
builder.append('}');
} else {
@@ -1160,7 +1193,7 @@ public class DDF implements Iterable<DDF> {
os.write(Integer.toString(type.getValue()).getBytes("UTF8"));
if (value != null) {
os.write(' ');
- encode(os, ((String) value).getBytes("ISO-8859-1"));
+ encode(os, (byte[]) value);
}
os.write('\n');
break;
@@ -1319,7 +1352,8 @@ public class DDF implements Iterable<DDF> {
// Unsafe string values are processed as ISO-8859-1.
// They may be anything, but it will guarantee a single byte encoding.
- return obj.unsafe_string(URLDecoder.decode(valueBuilder.toString(), "ISO-8859-1"));
+ return obj.unsafe_string(
+ URLDecoder.decode(valueBuilder.toString(), "ISO-8859-1").getBytes("ISO-8859-1"));
} catch (final IllegalArgumentException e) {
throw new IOException(e);
diff --git a/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequest.java b/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequest.java
index ac9fd9a..5b75523 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequest.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequest.java
@@ -23,6 +23,11 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.StringReader;
import java.io.UnsupportedEncodingException;
+import java.nio.ByteBuffer;
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.Charset;
+import java.nio.charset.CharsetDecoder;
+import java.nio.charset.CodingErrorAction;
import java.security.Principal;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
@@ -77,6 +82,18 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
/** Empty byte array for empty bodies. */
@Nonnull private static final byte[] EMPTY_BODY = new byte[0];
+ /** UTF-8 decoder. */
+ @Nonnull private static final CharsetDecoder UTF_8 =
+ Charset.forName("UTF-8").newDecoder()
+ .onMalformedInput(CodingErrorAction.REPORT)
+ .onUnmappableCharacter(CodingErrorAction.REPORT);
+
+ /** ISO single byte decoder. */
+ @Nonnull private static final CharsetDecoder ISO_8859_1 =
+ Charset.forName("ISO-8859-1").newDecoder()
+ .onMalformedInput(CodingErrorAction.REPORT)
+ .onUnmappableCharacter(CodingErrorAction.REPORT);
+
/** Underlying object containing remoted data. */
@Nonnull private final DDF obj;
@@ -207,7 +224,7 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
/** {@inheritDoc} */
public String getServerName() {
- return obj.getmember("hostname").string();
+ return decodeUnsafeString(obj.getmember("hostname").unsafe_string());
}
/** {@inheritDoc} */
@@ -383,12 +400,12 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
/** {@inheritDoc} */
public String getHeader(final String name) {
- return obj.getmember("headers").getmember(name).string();
+ return decodeUnsafeString(obj.getmember("headers").getmember(name).unsafe_string());
}
/** {@inheritDoc} */
public Enumeration<String> getHeaders(final String name) {
- final String s = obj.getmember("headers").getmember(name).string();
+ final String s = decodeUnsafeString(obj.getmember("headers").getmember(name).unsafe_string());
if (s != null) {
return Collections.enumeration(Collections.singletonList(s));
}
@@ -404,7 +421,7 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
public int getIntHeader(final String name) {
final DDF h = obj.getmember("headers").getmember(name);
if (h.isstring()) {
- return Integer.parseInt(h.string());
+ return Integer.parseInt(decodeUnsafeString(h.unsafe_string()));
}
return -1;
}
@@ -458,12 +475,12 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
/** {@inheritDoc} */
public String getRequestURI() {
- return obj.getmember("uri").string();
+ return decodeUnsafeString(obj.getmember("uri").unsafe_string());
}
/** {@inheritDoc} */
public StringBuffer getRequestURL() {
- final String url = obj.getmember("url").string();
+ final String url = decodeUnsafeString(obj.getmember("url").unsafe_string());
return new StringBuffer(url != null ? url : "");
}
@@ -540,6 +557,36 @@ public class RemotedHttpServletRequest implements HttpServletRequest {
public <T extends HttpUpgradeHandler> T upgrade(final Class<T> handlerClass) throws IOException, ServletException {
throw new UnsupportedOperationException();
}
+
+ /**
+ * Helper method to decode a byte buffer into either UTF-8 or ISO-8859-1.
+ *
+ * @param buffer input buffer
+ *
+ * @return encoded String form of the data
+ */
+ @Nullable private static String decodeUnsafeString(final byte[] buffer) {
+
+ if (buffer == null) {
+ return null;
+ }
+
+ final ByteBuffer wrapper = ByteBuffer.wrap(buffer);
+
+ try {
+ return UTF_8.decode(wrapper).toString();
+ } catch (final CharacterCodingException e) {
+
+ }
+
+ try {
+ return ISO_8859_1.decode(wrapper).toString();
+ } catch (final CharacterCodingException e) {
+
+ }
+
+ return null;
+ }
/** Helper class cribbed from Spring. */
private static class BodyInputStream extends ServletInputStream {
diff --git a/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponse.java b/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponse.java
index 4749548..4160cfc 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponse.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponse.java
@@ -241,7 +241,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
}
obj.getmember("response").remove();
- obj.addmember("redirect").unsafe_string(location);
+ obj.addmember("redirect").string(location);
committed = true;
outputStream = null;
}
@@ -267,7 +267,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
/** {@inheritDoc} */
public void addHeader(final String name, final String value) {
- getHeaderList().add(new DDF(name).unsafe_string(value));
+ getHeaderList().add(new DDF(name).string(value));
}
/** {@inheritDoc} */
@@ -383,6 +383,14 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
offset = 0;
}
+ @Nonnull private byte[] getBuffer() {
+ return buffer;
+ }
+
+ private int getOffset() {
+ return offset;
+ }
+
private boolean write(final int b) {
if (offset < buffer.length) {
buffer[offset++] = Integer.valueOf(b).byteValue();
@@ -391,12 +399,6 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
return false;
}
-
- private void flush(@Nonnull final StringBuffer sink) {
- for (int i = 0; i < offset; i++) {
- sink.appendCodePoint(buffer[i]);
- }
- }
}
private class BodyOutputStream extends ServletOutputStream {
@@ -436,9 +438,16 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
@Override
public void flush() throws IOException {
- final StringBuffer sink = new StringBuffer();
- bufferList.forEach(b -> b.flush(sink));
- obj.addmember("response.data").unsafe_string(sink.toString());
+
+ int offset = 0;
+ final byte[] copy = new byte[((bufferList.size() - 1) * bufferSize) +
+ bufferList.get(bufferList.size() - 1).getOffset()];
+
+ for (final ByteArrayWrapper b : bufferList) {
+ System.arraycopy(b.getBuffer(), 0, copy, offset, b.getOffset());
+ offset += b.getOffset();
+ }
+ obj.addmember("response.data").unsafe_string(copy);
committed = true;
}
diff --git a/src/test/java/net/shibboleth/utilities/java/support/ddf/DDFTest.java b/src/test/java/net/shibboleth/utilities/java/support/ddf/DDFTest.java
index 9845a30..d44cb68 100644
--- a/src/test/java/net/shibboleth/utilities/java/support/ddf/DDFTest.java
+++ b/src/test/java/net/shibboleth/utilities/java/support/ddf/DDFTest.java
@@ -23,6 +23,7 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
import javax.annotation.Nonnull;
@@ -33,7 +34,6 @@ import net.shibboleth.utilities.java.support.collection.Pair;
/**
* DDF unit tests.
*/
- at SuppressWarnings("javadoc")
public class DDFTest {
@Test
@@ -63,7 +63,7 @@ public class DDFTest {
}
@Test
- public void testConversions() {
+ public void testConversions() throws UnsupportedEncodingException {
final DDF obj = new DDF("foo");
obj.string("bar");
assertTrue(obj.isstring());
@@ -98,7 +98,7 @@ public class DDFTest {
assertEquals(obj.integer(), Integer.valueOf(42));
assertEquals(obj.floating(), Double.valueOf(42.42));
- obj.unsafe_string("bar");
+ obj.unsafe_string("bar".getBytes("ISO-8859-1"));
System.out.print(obj);
}
@@ -220,7 +220,7 @@ public class DDFTest {
sink.reset();
final byte[] unsafe = {102, 111, 111, -128, 98, 97, 114};
- obj.unsafe_string(new String(unsafe, "ISO-8859-1"));
+ obj.unsafe_string(unsafe);
obj.serialize(sink);
assertEquals(sink.toByteArray(), testFile("unsafestring-name.ddf"));
sink.reset();
@@ -285,10 +285,10 @@ public class DDFTest {
try (final InputStream is = getClass().getResourceAsStream("unsafestring-name.ddf")) {
final DDF obj = DDF.deserialize(is);
- assertTrue(obj.isstring());
+ assertTrue(obj.isunsafestring());
assertEquals(obj.name(), "foo bar");
final byte[] unsafe = {102, 111, 111, -128, 98, 97, 114};
- assertEquals(obj.string(), new String(unsafe, "ISO-8859-1"));
+ assertEquals(obj.unsafe_string(), unsafe);
}
try (final InputStream is = getClass().getResourceAsStream("int-name.ddf")) {
diff --git a/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequestTest.java b/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequestTest.java
index 2fccdc9..063a965 100644
--- a/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequestTest.java
+++ b/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletRequestTest.java
@@ -71,8 +71,9 @@ public class RemotedHttpServletRequestTest {
obj.addmember("port").integer(80);
obj.addmember("client_addr").string("127.0.0.1");
obj.addmember("remote_user").string("jdoe");
- obj.addmember("uri").string("/endpoint");
- obj.addmember("url").string("http://localhost/endpoint");
+ obj.addmember("hostname").unsafe_string("localhost".getBytes("UTF-8"));
+ obj.addmember("uri").unsafe_string("/endpoint".getBytes("UTF-8"));
+ obj.addmember("url").unsafe_string("http://localhost/endpoint".getBytes("UTF-8"));
obj.addmember("scheme").string("http");
assertEquals(req.getContentLength(), 100);
@@ -85,6 +86,7 @@ public class RemotedHttpServletRequestTest {
assertEquals(req.getQueryString(), null);
assertEquals(req.getRemoteAddr(), "127.0.0.1");
assertEquals(req.getRemoteUser(), "jdoe");
+ assertEquals(req.getServerName(), "localhost");
assertEquals(req.getRequestURI(), "/endpoint");
assertEquals(req.getRequestURL().toString(), "http://localhost/endpoint");
assertEquals(req.getScheme(), "http");
@@ -130,8 +132,8 @@ public class RemotedHttpServletRequestTest {
@Test
public void testHeaders() throws IOException {
obj.structure();
- obj.addmember("headers.foo").string("bar");
- obj.addmember("headers.zork").string("grue");
+ obj.addmember("headers.foo").unsafe_string("bar".getBytes("UTF-8"));
+ obj.addmember("headers.zork").unsafe_string("grue".getBytes("UTF-8"));
assertEquals(req.getHeaders("foo").nextElement(), "bar");
assertEquals(req.getHeader("zork"), "grue");
@@ -141,7 +143,7 @@ public class RemotedHttpServletRequestTest {
@Test
public void testCookie() throws IOException {
obj.structure();
- obj.addmember("headers.Cookie").string("foo=bar;");
+ obj.addmember("headers.Cookie").unsafe_string("foo=bar;".getBytes("UTF-8"));
final Cookie[] cookies = req.getCookies();
@@ -153,7 +155,7 @@ public class RemotedHttpServletRequestTest {
@Test
public void testCookies() throws IOException {
obj.structure();
- obj.addmember("headers.Cookie").string("foo=bar; zork=grue");
+ obj.addmember("headers.Cookie").unsafe_string("foo=bar; zork=grue".getBytes("UTF-8"));
final Cookie[] cookies = req.getCookies();
diff --git a/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponseTest.java b/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponseTest.java
index ab29db0..ef00bd7 100644
--- a/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponseTest.java
+++ b/src/test/java/net/shibboleth/utilities/java/support/ddf/RemotedHttpServletResponseTest.java
@@ -92,12 +92,12 @@ public class RemotedHttpServletResponseTest {
resp.setBufferSize(3);
resp.setStatus(200);
try (final OutputStream os = resp.getOutputStream()) {
- os.write("zorkmid".getBytes());
+ os.write("zorkmid".getBytes("UTF-8"));
}
assertTrue(resp.isCommitted());
assertEquals(obj.getmember("response.status").integer(), Integer.valueOf(200));
- assertEquals(obj.getmember("response.data").string(), "zorkmid");
+ assertEquals(obj.getmember("response.data").unsafe_string(), "zorkmid".getBytes("UTF-8"));
}
@Test
@@ -110,7 +110,7 @@ public class RemotedHttpServletResponseTest {
assertTrue(resp.isCommitted());
assertEquals(obj.getmember("response.status").integer(), Integer.valueOf(200));
- assertEquals(obj.getmember("response.data").string().getBytes("ISO-8859-1"), "zorkmid☯️".getBytes("ISO-8859-1"));
+ assertEquals(obj.getmember("response.data").unsafe_string(), "zorkmid☯️".getBytes("ISO-8859-1"));
}
@Test
@@ -123,10 +123,10 @@ public class RemotedHttpServletResponseTest {
assertTrue(resp.isCommitted());
assertEquals(obj.getmember("response.status").integer(), Integer.valueOf(200));
- assertEquals(obj.getmember("response.data").string(), "zorkmid");
+ assertEquals(obj.getmember("response.data").unsafe_string(), "zorkmid".getBytes("UTF-8"));
}
- @Test(enabled=false)
+ @Test
public void testWriter2() throws IOException {
resp.setBufferSize(3);
resp.setStatus(200);
@@ -136,6 +136,6 @@ public class RemotedHttpServletResponseTest {
assertTrue(resp.isCommitted());
assertEquals(obj.getmember("response.status").integer(), Integer.valueOf(200));
- assertEquals(obj.getmember("response.data").string(), "zorkmid☯️");
+ assertEquals(obj.getmember("response.data").unsafe_string(), "zorkmid☯️".getBytes("UTF-8"));
}
}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list