Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for Spring core integration #11288

Merged
merged 12 commits into from
Jan 26, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -668,4 +668,47 @@ public long getLongValue()
return _long;
}
}

public static class MultiHttpField extends HttpField
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
private final List<String> _list;

public MultiHttpField(String name, List<String> list)
{
super(name, buildValue(list));
_list = list;
}

private static String buildValue(List<String> list)
{
StringBuilder builder = null;
for (String v : list)
{
if (StringUtil.isBlank(v))
continue;
if (builder == null)
builder = new StringBuilder(list.size() * v.length() * 2);
else
builder.append(", ");
builder.append(v);
}

return builder == null ? null : builder.toString();
gregw marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public List<String> getValueList()
{
return _list;
}

@Override
public boolean contains(String search)
{
for (String v : _list)
if (StringUtil.asciiEqualsIgnoreCase(v, search))
return true;
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.ListIterator;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.BiPredicate;
Expand Down Expand Up @@ -989,6 +990,31 @@ default Mutable add(HttpFields fields)
return this;
}

/**
* Add a field with a list value;
gregw marked this conversation as resolved.
Show resolved Hide resolved
*
* @param name the name of the field
* @param list the List value of the field. If null the field is cleared.
* @return this builder
*/
default Mutable add(String name, List<String> list)
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
Objects.requireNonNull(name);
if (list == null || list.isEmpty())
return this;
if (list.size() == 1)
{
String v = list.get(0);
if (v == null)
return this;
return add(name, v);
}
HttpField field = new HttpField.MultiHttpField(name, list);
if (field.getValueList() == null)
gregw marked this conversation as resolved.
Show resolved Hide resolved
return this;
return add(field);
}

/**
* <p>Adds the given value(s) to the {@link HttpField} with the given name,
* encoding them as comma-separated if necessary,
Expand Down Expand Up @@ -1186,27 +1212,24 @@ default Mutable put(HttpHeader header, String value)
}

/**
* Set a field.
* Set a field to a list value;
gregw marked this conversation as resolved.
Show resolved Hide resolved
*
* @param name the name of the field
* @param list the List value of the field. If null the field is cleared.
* @return this builder
*/
default Mutable put(String name, List<String> list)
{
// TODO: this implementation should not add
// multiple headers, see RFC 9110 section 5.3.
boolean first = true;
for (String s : list)
{
HttpField field = new HttpField(name, s);
if (first)
put(field);
else
add(field);
first = false;
}
return this;
Objects.requireNonNull(name);
if (list == null || list.isEmpty())
return remove(name);
if (list.size() == 1)
return put(name, list.get(0));

HttpField field = new HttpField.MultiHttpField(name, list);
if (field.getValueList() == null)
gregw marked this conversation as resolved.
Show resolved Hide resolved
return remove(name);
return put(field);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.List;
import java.util.ListIterator;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.function.BiFunction;
import java.util.function.BiPredicate;
import java.util.stream.Stream;
Expand Down Expand Up @@ -358,20 +357,6 @@ public Mutable put(HttpHeader header, String value)
: put(new HttpField(header, value));
}

@Override
public Mutable put(String name, List<String> list)
{
Objects.requireNonNull(name);
Objects.requireNonNull(list);
remove(name);
for (String v : list)
{
if (v != null)
add(name, v);
}
return this;
}

@Override
public Mutable computeField(HttpHeader header, BiFunction<HttpHeader, List<HttpField>, HttpField> computeFn)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,13 +1000,62 @@ public void testPutNullName()
assertThat(fields.size(), is(0));
}

@Test
public void testAddNullValueList()
gregw marked this conversation as resolved.
Show resolved Hide resolved
{
HttpFields.Mutable fields = HttpFields.build();

fields.add("name", (List<String>)null);
assertThat(fields.size(), is(0));
List<String> list = new ArrayList<>();
fields.add("name", list);
assertThat(fields.size(), is(0));
list.add(null);
fields.add("name", list);
assertThat(fields.size(), is(0));
}

@Test
public void testAddValueList()
{
HttpFields.Mutable fields = HttpFields.build();

fields.add("name", "0, 1, 2");
fields.add("name", List.of("A", "B", "C"));
assertThat(fields.size(), is(2));
assertThat(fields.getValuesList("name"), contains("0, 1, 2", "A, B, C"));
assertThat(fields.getCSV("name", false), contains("0", "1", "2", "A", "B", "C"));
assertThat(fields.getField("name").getValueList(), contains("0", "1", "2"));
assertThat(fields.getField(1).getValueList(), contains("A", "B", "C"));
}

@Test
public void testPutNullValueList()
{
HttpFields.Mutable fields = HttpFields.build();

assertThrows(NullPointerException.class, () -> fields.put("name", (List<String>)null));
fields.add("name", "x");
fields.put("name", (List<String>)null);
assertThat(fields.size(), is(0));
List<String> list = new ArrayList<>();
fields.add("name", "x");
fields.put("name", list);
assertThat(fields.size(), is(0));
list.add(null);
fields.add("name", "x");
fields.put("name", list);
assertThat(fields.size(), is(0));
}

@Test
public void testPutValueList()
{
HttpFields.Mutable fields = HttpFields.build();

fields.put("name", List.of("A", "B", "C"));
assertThat(fields.size(), is(1));
assertThat(fields.get("name"), is("A, B, C"));
assertThat(fields.getField("name").getValueList(), contains("A", "B", "C"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void testEmptyHeaders() throws Exception

HttpFields.Mutable fields = HttpFields.build();
fields.add("Host", "something");
assertThrows(IllegalArgumentException.class, () -> fields.add("Null", null));
assertThrows(IllegalArgumentException.class, () -> fields.add("Null", (String)null));
gregw marked this conversation as resolved.
Show resolved Hide resolved
fields.add("Empty", "");
RequestInfo info = new RequestInfo("GET", "/index.html", fields);
assertFalse(gen.isChunking());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@

import java.nio.ByteBuffer;
import java.util.concurrent.Flow;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.thread.Invocable;

/**
* <p>A {@link Flow.Subscriber} that wraps a {@link Content.Sink}.
Expand All @@ -29,6 +31,7 @@ public class ContentSinkSubscriber implements Flow.Subscriber<Content.Chunk>
{
private final Content.Sink sink;
private final Callback callback;
private final AtomicInteger lastAndComplete = new AtomicInteger(2);
private Flow.Subscription subscription;

public ContentSinkSubscriber(Content.Sink sink, Callback callback)
Expand All @@ -49,22 +52,30 @@ public void onNext(Content.Chunk chunk)
{
// Retain the chunk because the write may not complete immediately.
chunk.retain();
// Always set last=false because we do the last write from onComplete().
sink.write(false, chunk.getByteBuffer(), Callback.from(() -> succeeded(chunk), x -> failed(chunk, x)));
}
sink.write(chunk.isLast(), chunk.getByteBuffer(), new Callback()
{
@Override
public InvocationType getInvocationType()
{
return Invocable.getInvocationType(callback);
}

private void succeeded(Content.Chunk chunk)
{
chunk.release();
if (!chunk.isLast())
subscription.request(1);
}
public void succeeded()
{
chunk.release();
if (chunk.isLast())
onComplete();
else
subscription.request(1);
}

private void failed(Content.Chunk chunk, Throwable failure)
{
chunk.release();
subscription.cancel();
onError(failure);
public void failed(Throwable failure)
{
chunk.release();
subscription.cancel();
onError(failure);
}
});
}

@Override
Expand All @@ -76,6 +87,8 @@ public void onError(Throwable failure)
@Override
public void onComplete()
{
sink.write(true, null, callback);
// wait until called twice: once from last write success and once from the publisher
if (lastAndComplete.decrementAndGet() == 0)
callback.succeeded();
gregw marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading