Copilot commented on code in PR #3019:
URL: https://github.com/apache/pekko/pull/3019#discussion_r3330303053
##########
stream/src/main/scala/org/apache/pekko/stream/impl/io/TlsGraphStage.scala:
##########
@@ -135,15 +129,20 @@ import pekko.util.ByteString
private def allocateBuffers(session: SSLSession): Unit = {
val packetSize = math.max(1, session.getPacketBufferSize)
val applicationSize = math.max(1, session.getApplicationBufferSize)
- val applicationBatchSize = applicationBufferSize(applicationSize,
packetSize)
transportPacketSize = packetSize
applicationRecordSize = applicationSize
- maxUserInBufferSize = applicationBatchSize
- maxUserOutBufferSize = applicationBatchSize
-
- transportOutBuffer = acquireTransportBuffer(packetSize)
- transportInBuffer = acquireTransportBuffer(packetSize)
- transportUnreadBuffer = acquireTransportBuffer(packetSize)
+ maxUserInBufferSize = applicationSize *
MaxApplicationRecordsPerEngineCall
+ maxUserOutBufferSize = applicationSize *
MaxApplicationRecordsPerEngineCall
+ maxTransportOutBufferSize = packetSize *
MaxApplicationRecordsPerEngineCall
Review Comment:
`maxUserInBufferSize`, `maxUserOutBufferSize`, and
`maxTransportOutBufferSize` are computed via Int multiplication. If
`SSLSession` ever reports large buffer sizes, these can silently overflow to a
negative value and later trigger invalid `ByteBuffer.allocate` sizes or
incorrect bounds. It’s safer to use overflow-checked multiplication so failures
are explicit and easier to diagnose.
##########
stream/src/main/scala/org/apache/pekko/stream/impl/io/TlsGraphStage.scala:
##########
@@ -567,15 +563,16 @@ import pekko.util.ByteString
}
private def growTransportOutBuffer(): Unit = {
- val oldBuffer = transportOutBuffer
- val oldCapacity = oldBuffer.capacity()
- if (oldCapacity > Int.MaxValue / 2)
- throw new IllegalStateException(s"Cannot grow TLS transport output
buffer beyond $oldCapacity bytes")
+ val oldCapacity = transportOutBuffer.capacity()
+ if (oldCapacity >= maxTransportOutBufferSize)
+ throw new IllegalStateException(
+ s"Cannot grow TLS transport output buffer beyond
$maxTransportOutBufferSize bytes")
- val bigger = acquireTransportBuffer(oldCapacity * 2)
+ // Double, but clamp to the cap so growth never overshoots
maxTransportOutBufferSize regardless
+ // of whether the cap is a power-of-two multiple of the packet size.
+ val bigger = ByteBuffer.allocate(math.min(maxTransportOutBufferSize,
oldCapacity * 2))
Review Comment:
`oldCapacity * 2` is evaluated as an `Int` and can overflow before being
clamped by `math.min(...)`, which could lead to a negative/incorrect allocation
size and a confusing failure mode. Compute the doubled size in `Long` (or use
`Math.multiplyExact`) before clamping to `maxTransportOutBufferSize`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
