Skip to content

Commit

Permalink
Issue #4794 HttpInput.setReadListener should check async state
Browse files Browse the repository at this point in the history
Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
janbartel committed Apr 21, 2020
1 parent f23236b commit 98403f1
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,10 @@ public void setReadListener(ReadListener readListener)

_listener = Objects.requireNonNull(readListener);

//illegal if async not started
if (!_channelState.isAsync())
throw new IllegalStateException("Async not started");

if (isError())
{
woken = _channelState.onReadReady();
Expand Down
181 changes: 130 additions & 51 deletions jetty-server/src/test/java/org/eclipse/jetty/server/HttpInputTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,69 +88,104 @@ public void failed(Throwable x)
super.failed(x);
}
}

@BeforeEach
public void before()

public class TestHttpInput extends HttpInput
{
_in = new HttpInput(new HttpChannelState(new HttpChannel(new MockConnector(), new HttpConfiguration(), null, null)
public TestHttpInput(HttpChannelState state)
{
@Override
public void onAsyncWaitForContent()
{
_history.add("asyncReadInterested");
}
})
super(state);
}

@Override
protected void produceContent() throws IOException
{
@Override
public void onReadUnready()
{
_history.add("s.onReadUnready");
super.onReadUnready();
}
_history.add("produceContent " + _fillAndParseSimulate.size());

@Override
public boolean onReadPossible()
for (String s = _fillAndParseSimulate.poll(); s != null; s = _fillAndParseSimulate.poll())
{
_history.add("s.onReadPossible");
return super.onReadPossible();
if ("_EOF_".equals(s))
_in.eof();
else
_in.addContent(new TContent(s));
}
}

@Override
public boolean onContentAdded()
{
_history.add("s.onDataAvailable");
return super.onContentAdded();
}
@Override
protected void blockForContent() throws IOException
{
_history.add("blockForContent");
super.blockForContent();
}
}

@Override
public boolean onReadReady()
{
_history.add("s.onReadReady");
return super.onReadReady();
}
})
public class TestHttpChannelState extends HttpChannelState
{
private boolean _fakeAsyncState;

public TestHttpChannelState(HttpChannel channel)
{
@Override
protected void produceContent() throws IOException
{
_history.add("produceContent " + _fillAndParseSimulate.size());

for (String s = _fillAndParseSimulate.poll(); s != null; s = _fillAndParseSimulate.poll())
{
if ("_EOF_".equals(s))
_in.eof();
else
_in.addContent(new TContent(s));
}
}
super(channel);
}

public boolean isFakeAsyncState()
{
return _fakeAsyncState;
}

public void setFakeAsyncState(boolean fakeAsyncState)
{
_fakeAsyncState = fakeAsyncState;
}

@Override
public boolean isAsync()
{
if (isFakeAsyncState())
return true;
return super.isAsync();
}

@Override
public void onReadUnready()
{
_history.add("s.onReadUnready");
super.onReadUnready();
}

@Override
public boolean onReadPossible()
{
_history.add("s.onReadPossible");
return super.onReadPossible();
}

@Override
public boolean onContentAdded()
{
_history.add("s.onDataAvailable");
return super.onContentAdded();
}

@Override
public boolean onReadReady()
{
_history.add("s.onReadReady");
return super.onReadReady();
}
}

@BeforeEach
public void before()
{
_in = new TestHttpInput(new TestHttpChannelState(new HttpChannel(new MockConnector(), new HttpConfiguration(), null, null)
{
@Override
protected void blockForContent() throws IOException
public void onAsyncWaitForContent()
{
_history.add("blockForContent");
super.blockForContent();
_history.add("asyncReadInterested");
}
};
})
);
}

@AfterEach
Expand Down Expand Up @@ -326,7 +361,9 @@ public void testBlockingEOF() throws Exception
@Test
public void testAsyncEmpty() throws Exception
{
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(true);
_in.setReadListener(_listener);
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(false);
assertThat(_history.poll(), equalTo("produceContent 0"));
assertThat(_history.poll(), equalTo("s.onReadUnready"));
assertThat(_history.poll(), nullValue());
Expand All @@ -341,7 +378,10 @@ public void testAsyncEmpty() throws Exception
@Test
public void testAsyncRead() throws Exception
{
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(true);
_in.setReadListener(_listener);
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(false);

assertThat(_history.poll(), equalTo("produceContent 0"));
assertThat(_history.poll(), equalTo("s.onReadUnready"));
assertThat(_history.poll(), nullValue());
Expand Down Expand Up @@ -388,7 +428,9 @@ public void testAsyncRead() throws Exception
@Test
public void testAsyncEOF() throws Exception
{
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(true);
_in.setReadListener(_listener);
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(false);
assertThat(_history.poll(), equalTo("produceContent 0"));
assertThat(_history.poll(), equalTo("s.onReadUnready"));
assertThat(_history.poll(), nullValue());
Expand All @@ -406,8 +448,10 @@ public void testAsyncEOF() throws Exception

@Test
public void testAsyncReadEOF() throws Exception
{
{
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(true);
_in.setReadListener(_listener);
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(false);
assertThat(_history.poll(), equalTo("produceContent 0"));
assertThat(_history.poll(), equalTo("s.onReadUnready"));
assertThat(_history.poll(), nullValue());
Expand Down Expand Up @@ -452,7 +496,9 @@ public void testAsyncReadEOF() throws Exception
@Test
public void testAsyncError() throws Exception
{
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(true);
_in.setReadListener(_listener);
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(false);
assertThat(_history.poll(), equalTo("produceContent 0"));
assertThat(_history.poll(), equalTo("s.onReadUnready"));
assertThat(_history.poll(), nullValue());
Expand All @@ -477,6 +523,39 @@ public void testAsyncError() throws Exception

assertThat(_history.poll(), nullValue());
}

@Test
public void testSetListenerWithNull() throws Exception
{
//test can't be null
assertThrows(NullPointerException.class, () ->
{
_in.setReadListener(null);
});
}

@Test
public void testSetListenerNotAsync() throws Exception
{
//test not async
assertThrows(IllegalStateException.class, () ->
{
_in.setReadListener(_listener);
});
}

@Test
public void testSetListenerAlreadySet() throws Exception
{
//test already set
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(true);
_in.setReadListener(_listener);
((TestHttpChannelState)_in.getHttpChannelState()).setFakeAsyncState(false);
assertThrows(IllegalStateException.class, () ->
{
_in.setReadListener(_listener);
});
}

@Test
public void testRecycle() throws Exception
Expand Down

0 comments on commit 98403f1

Please sign in to comment.