Skip to content

Commit

Permalink
Issue eclipse-ee4j#2016 Fixed TrailersTest
Browse files Browse the repository at this point in the history 
- if the queue size is not same as reserved space, and if it is last content
  and the content is a trailer, send trailers
- if the queue size is same as reserved space, and record was already processed,
  don't send trailers.
- TrailersTest - just formatting and finals

Signed-off-by: David Matějček <dmatej@seznam.cz>
  • Loading branch information
@dmatej
dmatej committed Nov 20, 2020
1 parent 8d5a37f commit 1d35017
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 28 deletions.
12 changes: 8 additions & 4 deletions modules/http2/src/main/java/org/glassfish/grizzly/http2/DefaultOutputSink.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ private OutputQueueRecord writeDownStream0(final HttpPacket httpPacket,
boolean sendTrailers = false;
boolean lockedByMe = false;
try {
// try-finally block to release deflater lock if needed
boolean isLast = httpContent != null && httpContent.isLast();
final boolean isTrailer = HttpTrailer.isTrailer(httpContent);

Expand Down Expand Up @@ -334,10 +333,15 @@ private OutputQueueRecord writeDownStream0(final HttpPacket httpPacket,
isLast, isZeroSizeData);
outputQueue.offer(record);

// check if our element wasn't forgotten (async)
if (outputQueue.size() != spaceToReserve || !outputQueue.remove(record)) {
// there is yet something in the queue before current record
if (outputQueue.size() != spaceToReserve) {
sendTrailers = isLast && isTrailer;
return null;
}
//
if (!outputQueue.remove(record)) {
sendTrailers = false;
LOGGER.finest("In some weird condition. FIXME... why are we removing what we added in previous if/else?");
LOGGER.finest("The record has been already processed.");
return null;
}
}
Expand Down
50 changes: 26 additions & 24 deletions modules/http2/src/test/java/org/glassfish/grizzly/http2/TrailersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.glassfish.grizzly.memory.Buffers;
import org.glassfish.grizzly.memory.MemoryManager;
import org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler;
import org.glassfish.grizzly.nio.transport.TCPNIOTransport;
import org.junit.After;
import org.junit.Test;

Expand All @@ -75,10 +76,10 @@ public void testTrailers() throws Exception {
configureHttpServer();
startHttpServer(new HttpHandler() {
@Override
public void service(Request request, Response response) throws Exception {
public void service(final Request request, final Response response) throws Exception {
response.setContentType("text/plain");
final InputStream in = request.getInputStream();
StringBuilder sb = new StringBuilder();
final StringBuilder sb = new StringBuilder();
int b;
while ((b = in.read()) != -1) {
sb.append((char) b);
Expand All @@ -103,7 +104,7 @@ public Map<String, String> get() {

final Filter filter = new BaseFilter() {
@Override
public NextAction handleRead(FilterChainContext ctx) throws IOException {
public NextAction handleRead(final FilterChainContext ctx) throws IOException {
final HttpContent httpContent = ctx.getMessage();
try {
if (lastProcessed.get()) {
Expand All @@ -120,7 +121,7 @@ public NextAction handleRead(FilterChainContext ctx) throws IOException {
latch.countDown();
} else {
assertFalse(httpContent instanceof HttpTrailer);
int result = contentCount.incrementAndGet();
final int result = contentCount.incrementAndGet();
if (result == 1) {
assertTrue(httpContent.getContent().remaining() == 0); // response
} else if (result == 2) {
Expand All @@ -129,7 +130,7 @@ public NextAction handleRead(FilterChainContext ctx) throws IOException {
fail("Unexpected content");
}
}
} catch (Throwable t) {
} catch (final Throwable t) {
error.set(t);
latch.countDown();
}
Expand All @@ -138,10 +139,11 @@ public NextAction handleRead(FilterChainContext ctx) throws IOException {
}
};
final Connection<?> c = getConnection("localhost", PORT, filter);
HttpRequestPacket.Builder builder = HttpRequestPacket.builder();
HttpRequestPacket request = builder.method(Method.POST).uri("/echo").protocol(Protocol.HTTP_2_0).host("localhost:" + PORT).build();
c.write(HttpTrailer.builder(request).content(Buffers.wrap(MemoryManager.DEFAULT_MEMORY_MANAGER, "a=b&c=d")).last(true).header("trailer-a", "value-a")
.header("trailer-b", "value-b").build());
final HttpRequestPacket.Builder builder = HttpRequestPacket.builder();
final HttpRequestPacket request = builder.method(Method.POST).uri("/echo")
.protocol(Protocol.HTTP_2_0).host("localhost:" + PORT).build();
c.write(HttpTrailer.builder(request).content(Buffers.wrap(MemoryManager.DEFAULT_MEMORY_MANAGER, "a=b&c=d"))
.last(true).header("trailer-a", "value-a").header("trailer-b", "value-b").build());
assertTrue(latch.await(10, TimeUnit.SECONDS));
final Throwable t = error.get();
if (t != null) {
Expand All @@ -156,7 +158,7 @@ public void testNoContentTrailers() throws Exception {
configureHttpServer();
startHttpServer(new HttpHandler() {
@Override
public void service(Request request, Response response) throws Exception {
public void service(final Request request, final Response response) throws Exception {
response.setContentType("text/plain");
final InputStream in = request.getInputStream();
// noinspection StatementWithEmptyBody
Expand All @@ -176,33 +178,34 @@ public Map<String, String> get() {

final Filter filter = new BaseFilter() {
@Override
public NextAction handleRead(FilterChainContext ctx) throws IOException {
public NextAction handleRead(final FilterChainContext ctx) throws IOException {
final HttpContent httpContent = ctx.getMessage();
try {
if (httpContent.isLast()) {
assertTrue(httpContent instanceof HttpTrailer);
final MimeHeaders trailers = ((HttpTrailer) httpContent).getHeaders();

assertEquals(2, trailers.size());
assertEquals("value-a", trailers.getHeader("trailer-a"));
assertEquals("value-b", trailers.getHeader("trailer-b"));
latch.countDown();
}
} catch (Throwable t) {
} catch (final Throwable t) {
error.set(t);
} finally {
latch.countDown();
}

return ctx.getStopAction();
}
};
final Connection<?> c = getConnection("localhost", PORT, filter);
HttpRequestPacket.Builder builder = HttpRequestPacket.builder();
HttpRequestPacket request = builder.method(Method.POST).uri("/echo").protocol(Protocol.HTTP_2_0).host("localhost:" + PORT).build();
c.write(HttpContent.builder(request) // write the request
.last(false).build());
c.write(HttpTrailer.builder(request) // write the trailer
.content(Buffers.EMPTY_BUFFER).last(true).header("trailer-a", "value-a").header("trailer-b", "value-b").build());
final HttpRequestPacket.Builder builder = HttpRequestPacket.builder();
final HttpRequestPacket request = builder.method(Method.POST).uri("/echo")
.protocol(Protocol.HTTP_2_0).host("localhost:" + PORT).build();
// write the request
c.write(HttpContent.builder(request).last(false).build());
// write the trailer
c.write(HttpTrailer.builder(request).content(Buffers.EMPTY_BUFFER).last(true)
.header("trailer-a", "value-a").header("trailer-b", "value-b").build());
assertTrue(latch.await(10, TimeUnit.SECONDS));
final Throwable t = error.get();
if (t != null) {
Expand All @@ -227,10 +230,9 @@ private Connection getConnection(final String host, final int port, final Filter

final FilterChain clientChain = createClientFilterChainAsBuilder(false, true, clientFilter).build();

SocketConnectorHandler connectorHandler = TCPNIOConnectorHandler.builder(httpServer.getListener("grizzly").getTransport()).processor(clientChain)
.build();

Future<Connection> connectFuture = connectorHandler.connect(host, port);
final TCPNIOTransport transport = httpServer.getListener("grizzly").getTransport();
final SocketConnectorHandler connectorHandler = TCPNIOConnectorHandler.builder(transport).processor(clientChain).build();
final Future<Connection> connectFuture = connectorHandler.connect(host, port);
return connectFuture.get(10, TimeUnit.SECONDS);

}
Expand Down

0 comments on commit 1d35017

Please sign in to comment.