-
Notifications
You must be signed in to change notification settings - Fork 53
Add server-side WebSocket compression #1114
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| # Add server-side WebSocket compression support and per-upgrade compression control. | ||
| ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.impl.engine.ws.Handshake#Server.buildResponse") | ||
| ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.pekko.http.javadsl.model.ws.WebSocketUpgrade.handleMessagesWith") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -348,6 +348,44 @@ pekko.http { | |
|
|
||
| # Enable verbose debug logging for all ingoing and outgoing frames | ||
| log-frames = false | ||
|
|
||
| compression { | ||
| # Whether the server should support WebSocket compression using the RFC 7692 | ||
| # permessage-deflate extension. Compression is negotiated during the | ||
| # WebSocket handshake and is only used when the client requests it. | ||
| enabled = true | ||
|
|
||
| # Maximum size of a decompressed WebSocket message. If this value is | ||
| # exceeded while inflating a compressed message, the connection is closed | ||
| # with a WebSocket protocol error. | ||
| # Set to 0 to disable this limit. | ||
| max-allocation = 64k | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 64 KB feels quite conservative for a default. Real-world WebSocket apps (collaborative editing, gaming, large JSON payloads) routinely exceed this. Tomcat has no limit by default, Jetty uses 128 KB for text message size. Something like 256 KB or 1 MB might be a more practical default while still protecting against decompression bombs. Users who need tighter limits can always lower it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be ok with 256 KB - 1 MB seems a bit too much.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no strong preference. 256Kb can be the starting point. If we get requests to increase the default later on, we can do that. The value is a configurable but a default setting that only some people need to worry about is ideal. |
||
|
|
||
| permessage-deflate { | ||
| # Pekko HTTP uses the JDK Deflater/Inflater implementation for | ||
| # permessage-deflate. The JDK API does not expose zlib windowBits or | ||
| # memLevel, so clients cannot negotiate a server_max_window_bits value | ||
| # below 15. client_max_window_bits can still be negotiated through | ||
| # preferred-client-window-size. | ||
|
|
||
| # DEFLATE compression level used for server-to-client messages. Valid | ||
| # values are 0-9, where 0 uses no compression and 9 favors compression | ||
| # ratio over CPU usage. | ||
| compression-level = 6 | ||
|
|
||
| # The client_max_window_bits value Pekko HTTP should request for | ||
| # client-to-server messages when the client sends client_max_window_bits | ||
| # without an explicit value. Valid values are 8-15. | ||
| preferred-client-window-size = 15 | ||
|
|
||
| # Whether a client may request server_no_context_takeover. | ||
| allow-server-no-context = false | ||
|
|
||
| # Whether Pekko HTTP should request client_no_context_takeover when the | ||
| # client indicates that it supports this parameter. | ||
| preferred-client-no-context = false | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import scala.collection.immutable.Seq | |
| import pekko.event.LoggingAdapter | ||
| import pekko.http.impl.util._ | ||
| import pekko.http.impl.engine.server.UpgradeToOtherProtocolResponseHeader | ||
| import pekko.http.impl.settings.WebSocketSettingsImpl | ||
| import pekko.http.scaladsl.model.headers._ | ||
| import pekko.http.scaladsl.model.ws.Message | ||
| import pekko.http.scaladsl.model._ | ||
|
|
@@ -122,26 +123,47 @@ private[http] object Handshake { | |
| case OptionVal.Some(p) => p.protocols | ||
| case _ => Nil | ||
| } | ||
| val clientRequestedExtensions = headers.collect { | ||
| case extensions: `Sec-WebSocket-Extensions` => extensions.extensions | ||
| }.flatten | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to avoid the flatten and do it in one call? |
||
| val perMessageDeflate = | ||
| PerMessageDeflate.negotiate( | ||
| clientRequestedExtensions, | ||
| settings.asInstanceOf[WebSocketSettingsImpl].compression) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This unchecked cast will throw a |
||
|
|
||
| val header = new UpgradeToWebSocketLowLevel { | ||
| def requestedProtocols: Seq[String] = clientSupportedSubprotocols | ||
|
|
||
| def handle( | ||
| handler: Either[Graph[FlowShape[FrameEvent, FrameEvent], Any], Graph[FlowShape[Message, Message], Any]], | ||
| subprotocol: Option[String]): HttpResponse = { | ||
| subprotocol: Option[String], | ||
| compressionEnabled: Boolean): HttpResponse = { | ||
| require( | ||
| subprotocol.forall(chosen => clientSupportedSubprotocols.contains(chosen)), | ||
| s"Tried to choose invalid subprotocol '$subprotocol' which wasn't offered by the client: [${requestedProtocols.mkString(", ")}]") | ||
| buildResponse(key.get, handler, subprotocol, settings, log) | ||
| val acceptedPerMessageDeflate = if (compressionEnabled) perMessageDeflate else None | ||
| buildResponse(key.get, handler, subprotocol, acceptedPerMessageDeflate, settings, log) | ||
| } | ||
|
|
||
| def handleFrames( | ||
| handlerFlow: Graph[FlowShape[FrameEvent, FrameEvent], Any], subprotocol: Option[String]): HttpResponse = | ||
| handle(Left(handlerFlow), subprotocol) | ||
| handle(Left(handlerFlow), subprotocol, compressionEnabled = true) | ||
|
|
||
| override private[http] def handleFrames( | ||
| handlerFlow: Graph[FlowShape[FrameEvent, FrameEvent], Any], | ||
| subprotocol: Option[String], | ||
| compressionEnabled: Boolean): HttpResponse = | ||
| handle(Left(handlerFlow), subprotocol, compressionEnabled) | ||
|
|
||
| override def handleMessages(handlerFlow: Graph[FlowShape[Message, Message], Any], | ||
| subprotocol: Option[String] = None): HttpResponse = | ||
| handle(Right(handlerFlow), subprotocol) | ||
| handle(Right(handlerFlow), subprotocol, compressionEnabled = true) | ||
|
|
||
| override def handleMessages( | ||
| handlerFlow: Graph[FlowShape[Message, Message], Any], | ||
| subprotocol: Option[String], | ||
| compressionEnabled: Boolean): HttpResponse = | ||
| handle(Right(handlerFlow), subprotocol, compressionEnabled) | ||
| } | ||
| OptionVal.Some(header) | ||
| } else OptionVal.None | ||
|
|
@@ -169,11 +191,16 @@ private[http] object Handshake { | |
| */ | ||
| def buildResponse(key: `Sec-WebSocket-Key`, | ||
| handler: Either[Graph[FlowShape[FrameEvent, FrameEvent], Any], Graph[FlowShape[Message, Message], Any]], | ||
| subprotocol: Option[String], settings: WebSocketSettings, log: LoggingAdapter): HttpResponse = { | ||
| subprotocol: Option[String], | ||
| perMessageDeflate: Option[PerMessageDeflate.Negotiated], | ||
| settings: WebSocketSettings, | ||
| log: LoggingAdapter): HttpResponse = { | ||
| val frameHandler = handler match { | ||
| case Left(frameHandler) => frameHandler | ||
| case Left(frameHandler) => | ||
| perMessageDeflate.map(_.frameEventBidiFlow(settings.randomFactory).join(frameHandler)).getOrElse(frameHandler) | ||
| case Right(messageHandler) => | ||
| WebSocket.stack(serverSide = true, settings, log = log).join(messageHandler) | ||
| WebSocket.stack(serverSide = true, settings, perMessageDeflate = perMessageDeflate, log = log) | ||
| .join(messageHandler) | ||
| } | ||
|
|
||
| HttpResponse( | ||
|
|
@@ -182,7 +209,9 @@ private[http] object Handshake { | |
| List( | ||
| UpgradeHeader, | ||
| ConnectionUpgradeHeader, | ||
| `Sec-WebSocket-Accept`.forKey(key), | ||
| `Sec-WebSocket-Accept`.forKey(key)) ::: | ||
| perMessageDeflate.map(p => `Sec-WebSocket-Extensions`(Seq(p.responseExtension))).toList ::: | ||
| List( | ||
| UpgradeToOtherProtocolResponseHeader(WebSocket.framing.join(frameHandler)))) | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do adaptive compression?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify what kind of adaptive compression you mean?
With this PR, Pekko HTTP already supports adaptation in two ways:
permessage-deflate.compressionEnabledoverloads.If you mean adapting per outbound message, for example compressing a large message but sending a small message uncompressed, I think that could make sense as a follow-up PR.
permessage-deflateallows this: once the extension is negotiated, a sender can still choose to leave RSV1 unset for individual messages. This would need careful handling around fragmented messages and context takeover, but a minimum-size threshold or application-controlled filter could be useful.In Play we do basically the same for the gzip filter: https://github.com/playframework/playframework/blob/d87116f880789e0dbc40b7f971e08638e5ab7557/web/play-filters-helpers/src/main/resources/reference.conf#L297-L300 - which avoids gziping content that is smaller then a certain threshold.
But, as said, I would do that in a follow up PR if desired.
Similarly, skipping compression for certain message types or already-compressed payloads could make sense later, but Pekko HTTP currently does not know application-level message semantics at this layer, so that would also need separate design.
If you mean changing the compression level dynamically, that is more complicated and probably less useful as a general automatic feature. A simpler and more realistic variant would be allowing applications to choose a compression level per WebSocket connection, similar to how this PR allows enabling/disabling compression per accepted WebSocket. That should be much easier to reason about than changing the level message-by-message.
I would also do that in a follow up PR if desired.
If you mean disabling compression automatically under CPU or memory pressure, I think that is much larger in scope and probably not a good fit for this PR. It would need metrics, policy, and predictable behavior, and I am not sure the added complexity would be worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some research if netty has similiar feature(s):
What Netty does have is lower-level extension filters:
Those filters can decide per frame whether an extension encoder/decoder should be skipped. Netty’s own tests show a threshold-style example:
So Netty supports the mechanism for "don’t compress small messages", but it does not ship a named adaptive compression feature/config. Its default is
WebSocketExtensionFilterProvider.DEFAULT, which meansNEVER_SKIP, so negotiated messages are compressed.Important detail: for
permessage-deflate, Netty forbids changing the decision in the middle of a fragmented message. If compression/decompression is already in progress and the filter suddenly says “skip” for a continuation frame, Netty throws.So yeah, maybe in a follow up PR it might make sense to think about some mechanism to dynamicaly decide if an outbound message should be compressed or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but that can come up later, we are using something like this later.