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

Issue #4747 - fix some test failures for jetty-10 with websocket tck #4750

Merged
merged 10 commits into from
Apr 23, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,6 @@ else if (values.length == 1)

// Verify the negotiated subprotocol
List<String> offeredSubProtocols = getSubProtocols();
if (negotiatedSubProtocol == null && !offeredSubProtocols.isEmpty())
throw new WebSocketException("Upgrade failed: no subprotocol selected from offered subprotocols ");
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
if (negotiatedSubProtocol != null && !offeredSubProtocols.contains(negotiatedSubProtocol))
throw new WebSocketException("Upgrade failed: subprotocol [" + negotiatedSubProtocol + "] not found in offered subprotocols " + offeredSubProtocols);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,7 @@ private class IncomingAdaptor implements IncomingFrames
@Override
public void onFrame(Frame frame, final Callback callback)
{
Callback closeCallback = null;
try
{
if (LOG.isDebugEnabled())
Expand All @@ -695,11 +696,13 @@ public void onFrame(Frame frame, final Callback callback)

// Handle inbound CLOSE
connection.cancelDemand();
Callback closeCallback;

if (closeConnection)
{
closeCallback = Callback.from(() -> closeConnection(sessionState.getCloseStatus(), callback));
closeCallback = Callback.from(() -> closeConnection(sessionState.getCloseStatus(), callback), t ->
{
sessionState.onError(t);
closeConnection(sessionState.getCloseStatus(), callback);
});
}
else
{
Expand All @@ -725,7 +728,10 @@ public void onFrame(Frame frame, final Callback callback)
}
catch (Throwable t)
{
callback.failed(t);
if (closeCallback != null)
closeCallback.failed(t);
else
callback.failed(t);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.nio.channels.ClosedChannelException;

import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.CloseStatus;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.OpCode;
Expand Down Expand Up @@ -123,6 +124,39 @@ public boolean onClosed(CloseStatus closeStatus)
}
}

/**
* <p>
* If no error is set in the CloseStatus this will either, replace the current close status with
* a {@link CloseStatus#SERVER_ERROR} status if we had a NORMAL close code, or, it will set the cause
* of the CloseStatus if the previous cause was null, this allows onError to be notified after the connection is closed.
* </p>
* <p>
* This should only be called if there is an error directly before the call to
* {@link WebSocketCoreSession#closeConnection(CloseStatus, Callback)}.
* </p>
* <p>
* This could occur if the FrameHandler throws an exception in onFrame after receiving a close frame reply, in this
* case to notify onError we must set the cause in the closeStatus.
* </p>
* @param t the error which occurred.
*/
public void onError(Throwable t)
{
synchronized (this)
{
if (_sessionState != State.CLOSED || _closeStatus == null)
throw new IllegalArgumentException();

// Override any normal close status.
if (!_closeStatus.isAbnormal())
_closeStatus = new CloseStatus(CloseStatus.SERVER_ERROR, t);

// Otherwise set the error if it wasn't already set to notify onError as well as onClose.
if (_closeStatus.getCause() == null)
_closeStatus = new CloseStatus(_closeStatus.getCode(), _closeStatus.getReason(), t);
}
}

public boolean onEof()
{
synchronized (this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,14 @@ public boolean upgradeRequest(WebSocketNegotiator negotiator, HttpServletRequest
return false;
}

// Validate negotiated protocol
// Validate negotiated protocol.
String protocol = negotiation.getSubprotocol();
List<String> offeredProtocols = negotiation.getOfferedSubprotocols();
if (protocol != null)
{
if (!offeredProtocols.contains(protocol))
throw new WebSocketException("not upgraded: selected a protocol not present in offered protocols");
}
else
{
if (!offeredProtocols.isEmpty())
throw new WebSocketException("not upgraded: no protocol selected from offered protocols");
}

// validate negotiated extensions
for (ExtensionConfig config : negotiation.getNegotiatedExtensions())
Expand Down
Loading