Merge pull request #926 from danicheg/merge-series/0.23-into-main

Merge `series/0.23` into `main`

.github/workflows/ci.yml

@@ -34,15 +34,18 @@ jobs:
     runs-on: ${{ matrix.os }}
     timeout-minutes: 60
     steps:
+      - name: Install sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Checkout current branch (full)
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: 8
@@ -91,7 +94,7 @@ jobs:
 
       - name: Upload target directories
         if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: target-${{ matrix.os }}-${{ matrix.java }}-${{ matrix.scala }}
           path: targets.tar
@@ -106,15 +109,18 @@ jobs:
         java: [temurin@8]
     runs-on: ${{ matrix.os }}
     steps:
+      - name: Install sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Checkout current branch (full)
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: 8
@@ -125,7 +131,7 @@ jobs:
         run: sbt +update
 
       - name: Download target directories (3)
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: target-${{ matrix.os }}-${{ matrix.java }}-3
 
@@ -135,7 +141,7 @@ jobs:
           rm targets.tar
 
       - name: Download target directories (2.13)
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: target-${{ matrix.os }}-${{ matrix.java }}-2.13
 
@@ -149,15 +155,15 @@ jobs:
         env:
           PGP_SECRET: ${{ secrets.PGP_SECRET }}
           PGP_PASSPHRASE: ${{ secrets.PGP_PASSPHRASE }}
-        run: echo $PGP_SECRET | base64 -di | gpg --import
+        run: echo $PGP_SECRET | base64 -d -i - | gpg --import
 
       - name: Import signing key and strip passphrase
         if: env.PGP_SECRET != '' && env.PGP_PASSPHRASE != ''
         env:
           PGP_SECRET: ${{ secrets.PGP_SECRET }}
           PGP_PASSPHRASE: ${{ secrets.PGP_PASSPHRASE }}
         run: |
-          echo "$PGP_SECRET" | base64 -di > /tmp/signing-key.gpg
+          echo "$PGP_SECRET" | base64 -d -i - > /tmp/signing-key.gpg
           echo "$PGP_PASSPHRASE" | gpg --pinentry-mode loopback --passphrase-fd 0 --import /tmp/signing-key.gpg
           (echo "$PGP_PASSPHRASE"; echo; echo) | gpg --command-fd 0 --pinentry-mode loopback --change-passphrase $(gpg --list-secret-keys --with-colons 2> /dev/null | grep '^sec:' | cut --delimiter ':' --fields 5 | tail -n 1)
 
@@ -170,22 +176,25 @@ jobs:
 
   dependency-submission:
     name: Submit Dependencies
-    if: github.event_name != 'pull_request'
+    if: github.event.repository.fork == false && github.event_name != 'pull_request'
     strategy:
       matrix:
         os: [ubuntu-latest]
         java: [temurin@8]
     runs-on: ${{ matrix.os }}
     steps:
+      - name: Install sbt
+        uses: sbt/setup-sbt@v1
+
       - name: Checkout current branch (full)
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
       - name: Setup Java (temurin@8)
         id: setup-java-temurin-8
         if: matrix.java == 'temurin@8'
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: 8
@@ -210,12 +219,12 @@ jobs:
     runs-on: ${{ matrix.os }}
     steps:
       - name: Checkout current branch (fast)
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Setup Java (temurin@11)
         id: setup-java-temurin-11
         if: matrix.java == 'temurin@11'
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: 11

blaze-client/src/main/scala/org/http4s/blaze/client/Http1Connection.scala

@@ -31,6 +31,7 @@ import org.http4s.Uri.RegName
 import org.http4s.blaze.core.Http1Stage
 import org.http4s.blaze.core.IdleTimeoutStage
 import org.http4s.blaze.core.util.Http1Writer
+import org.http4s.blaze.core.util.NullWriter
 import org.http4s.blaze.pipeline.Command.EOF
 import org.http4s.client.RequestKey
 import org.http4s.headers.Host
@@ -200,10 +201,8 @@ private final class Http1Connection[F[_]](
           if (userAgent.nonEmpty && !req.headers.contains[`User-Agent`])
             rr << userAgent.get << "\r\n"
 
-          val mustClose: Boolean = req.headers.get[HConnection] match {
-            case Some(conn) => checkCloseConnection(conn, rr)
-            case None => getHttpMinor(req) == 0
-          }
+          val mustClose: Boolean =
+            checkRequestCloseConnection(req.headers.get[HConnection], getHttpMinor(req), NullWriter)
 
           val writeRequest: F[Boolean] = getChunkEncoder(req, mustClose, rr)
             .write(rr, req.entity)

blaze-client/src/test/scala/org/http4s/blaze/client/BlazeClientBase.scala

@@ -20,7 +20,6 @@ package client
 import cats.effect._
 import cats.effect.kernel.Resource
 import cats.effect.std.Dispatcher
-import cats.implicits.catsSyntaxApplicativeId
 import cats.syntax.all._
 import fs2.Stream
 import io.netty.channel.ChannelHandlerContext
@@ -141,9 +140,9 @@ trait BlazeClientBase extends CatsEffectSuite {
     )
 
   val server: IOFixture[ServerScaffold[IO]] =
-    ResourceSuiteLocalFixture("http", makeScaffold(2, false))
+    ResourceSuiteLocalFixture("http", makeScaffold(2, secure = false))
   val secureServer: IOFixture[ServerScaffold[IO]] =
-    ResourceSuiteLocalFixture("https", makeScaffold(1, true))
+    ResourceSuiteLocalFixture("https", makeScaffold(1, secure = true))
 
   override val munitFixtures = List(
     server,

blaze-client/src/test/scala/org/http4s/blaze/client/BlazeClientSuite.scala

@@ -317,7 +317,7 @@ class BlazeClientSuite extends BlazeClientBase {
         }
         override def onRequestEnd(ctx: ChannelHandlerContext, request: HttpRequest): Unit = ()
       })
-      ServerScaffold[IO](1, false, HandlersToNettyAdapter[IO](handlers)).use { server =>
+      ServerScaffold[IO](1, secure = false, HandlersToNettyAdapter[IO](handlers)).use { server =>
         val address = server.addresses.head
         val name = address.host
         val port = address.port
@@ -343,7 +343,7 @@ class BlazeClientSuite extends BlazeClientBase {
         }
         override def onRequestEnd(ctx: ChannelHandlerContext, request: HttpRequest): Unit = ()
       })
-      ServerScaffold[IO](1, false, HandlersToNettyAdapter[IO](handlers)).use { server =>
+      ServerScaffold[IO](1, secure = false, HandlersToNettyAdapter[IO](handlers)).use { server =>
         val address = server.addresses.head
         val name = address.host
         val port = address.port
@@ -366,7 +366,7 @@ class BlazeClientSuite extends BlazeClientBase {
         }
         override def onRequestEnd(ctx: ChannelHandlerContext, request: HttpRequest): Unit = ()
       })
-      ServerScaffold[IO](1, false, HandlersToNettyAdapter[IO](handlers)).use { server =>
+      ServerScaffold[IO](1, secure = false, HandlersToNettyAdapter[IO](handlers)).use { server =>
         val address = server.addresses.head
         val name = address.host
         val port = address.port

blaze-client/src/test/scala/org/http4s/blaze/client/Http1ClientStageSuite.scala

@@ -22,7 +22,6 @@ import cats.effect._
 import cats.effect.kernel.Deferred
 import cats.effect.std.Dispatcher
 import cats.effect.std.Queue
-import cats.syntax.all._
 import fs2.Stream
 import munit.CatsEffectSuite
 import org.http4s.BuildInfo

blaze-core/src/main/scala/org/http4s/blaze/core/Http1Stage.scala

@@ -67,6 +67,7 @@ private[blaze] trait Http1Stage[F[_]] { self: TailStage[ByteBuffer] =>
   protected def contentComplete(): Boolean
 
   /** Check Connection header and add applicable headers to response */
+  @deprecated("Use checkConnectionPersistence(Option[Connection], Int, Writer) instead", "0.23.17")
   protected final def checkCloseConnection(conn: Connection, rr: StringWriter): Boolean =
     if (conn.hasKeepAlive) { // connection, look to the request
       logger.trace("Found Keep-Alive header")
@@ -83,6 +84,42 @@ private[blaze] trait Http1Stage[F[_]] { self: TailStage[ByteBuffer] =>
       true
     }
 
+  /** Checks whether the connection should be closed per the request's Connection header
+    * and the HTTP version.
+    *
+    * As a side effect, writes a "Connection: close" header to the StringWriter if
+    * the request explicitly requests the connection is closed.
+    *
+    * @see [[https://datatracker.ietf.org/doc/html/rfc9112#name-persistence RFC 9112, Section 9.3, Persistence]]
+    */
+  private[http4s] final def checkRequestCloseConnection(
+      conn: Option[Connection],
+      minorVersion: Int,
+      rr: Writer,
+  ): Boolean =
+    if (conn.fold(false)(_.hasClose)) {
+      logger.trace(s"Closing ${conn} due to explicit close option in request's Connection header")
+      // This side effect doesn't really belong here, but is relied
+      // upon in multiple places as we look for the encoder.  The
+      // related problem of writing the keep-alive header in HTTP/1.0
+      // is handled elsewhere.
+      if (minorVersion >= 1) {
+        rr << "Connection: close\r\n"
+      }
+      true
+    } else if (minorVersion >= 1) {
+      logger.trace(s"Keeping ${conn} alive per default behavior of HTTP >= 1.1")
+      false
+    } else if (conn.fold(false)(_.hasKeepAlive)) {
+      logger.trace(
+        s"Keeping ${conn} alive due to explicit keep-alive option in request's Connection header"
+      )
+      false
+    } else {
+      logger.trace(s"Closing ${conn} per default behavior of HTTP/1.0")
+      true
+    }
+
   /** Get the proper body encoder based on the request */
   protected final def getEncoder(
       req: Request[F],

blaze-core/src/main/scala/org/http4s/blaze/core/util/CachingChunkWriter.scala

@@ -103,7 +103,7 @@ private[blaze] class CachingChunkWriter[F[_]](
           pipe.channelWrite(hbuff)
         }
       } else if (!chunk.isEmpty) {
-        writeBodyChunk(chunk, true).flatMap { _ =>
+        writeBodyChunk(chunk, flush = true).flatMap { _ =>
           writeTrailer(pipe, trailer)
         }
       } else {

blaze-core/src/main/scala/org/http4s/blaze/core/util/FlushingChunkWriter.scala

@@ -40,7 +40,7 @@ private[blaze] class FlushingChunkWriter[F[_]](pipe: TailStage[ByteBuffer], trai
     else pipe.channelWrite(encodeChunk(chunk, Nil))
 
   protected def writeEnd(chunk: Chunk[Byte]): Future[Boolean] = {
-    if (!chunk.isEmpty) writeBodyChunk(chunk, true).flatMap { _ =>
+    if (!chunk.isEmpty) writeBodyChunk(chunk, flush = true).flatMap { _ =>
       writeTrailer(pipe, trailer)
     }
     else writeTrailer(pipe, trailer)

blaze-core/src/main/scala/org/http4s/blaze/core/util/NullWriter.scala

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2014 http4s.org
+ *
+ * Licensed 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.
+ */
+
+package org.http4s
+package blaze.core.util
+
+import org.http4s.util.Writer
+
+/** A writer that does not write.  Not to be confused with an
+  * [[EntityBodyWriter]].
+  */
+private[http4s] object NullWriter extends Writer {
+  def append(s: String): NullWriter.type = NullWriter
+}

blaze-core/src/test/scala/org/http4s/blaze/core/util/Http1WriterSpec.scala

@@ -133,13 +133,25 @@ class Http1WriterSpec extends CatsEffectSuite with DispatcherIOFixture {
   runNonChunkedTests(
     "CachingChunkWriter",
     implicit dispatcher =>
-      tail => new CachingChunkWriter[IO](tail, IO.pure(Headers.empty), 1024 * 1024, false),
+      tail =>
+        new CachingChunkWriter[IO](
+          tail,
+          IO.pure(Headers.empty),
+          1024 * 1024,
+          omitEmptyContentLength = false,
+        ),
   )
 
   runNonChunkedTests(
     "CachingStaticWriter",
     implicit dispatcher =>
-      tail => new CachingChunkWriter[IO](tail, IO.pure(Headers.empty), 1024 * 1024, false),
+      tail =>
+        new CachingChunkWriter[IO](
+          tail,
+          IO.pure(Headers.empty),
+          1024 * 1024,
+          omitEmptyContentLength = false,
+        ),
   )
 
   def builder(tail: TailStage[ByteBuffer])(implicit D: Dispatcher[IO]): FlushingChunkWriter[IO] =

blaze-server/src/main/scala/org/http4s/blaze/server/Http1ServerStage.scala

@@ -245,12 +245,9 @@ private[blaze] class Http1ServerStage[F[_]](
     // Need to decide which encoder and if to close on finish
     val closeOnFinish = respConn
       .map(_.hasClose)
-      .orElse {
-        req.headers.get[Connection].map(checkCloseConnection(_, rr))
-      }
       .getOrElse(
-        parser.minorVersion() == 0
-      ) // Finally, if nobody specifies, http 1.0 defaults to close
+        checkRequestCloseConnection(req.headers.get[Connection], parser.minorVersion(), rr)
+      )
 
     // choose a body encoder. Will add a Transfer-Encoding header if necessary
     val bodyEncoder: Http1Writer[F] =
@@ -289,7 +286,7 @@ private[blaze] class Http1ServerStage[F[_]](
           rr,
           parser.minorVersion(),
           closeOnFinish,
-          false,
+          omitEmptyContentLength = false,
         )
 
     // TODO: pool shifting: https://github.com/http4s/http4s/blob/main/core/src/main/scala/org/http4s/internal/package.scala#L45

blaze-server/src/main/scala/org/http4s/blaze/server/WebSocketSupport.scala

@@ -116,7 +116,7 @@ private[http4s] trait WebSocketSupport[F[_]] extends Http1ServerStage[F] {
                       .prepend(new WSFrameAggregator)
                       .prepend(new WebSocketDecoder(maxBufferSize.getOrElse(0)))
 
-                  this.replaceTail(segment, true)
+                  this.replaceTail(segment, startup = true)
 
                 case Failure(t) => fatalError(t, "Error writing Websocket upgrade response")
               }(executionContext)

blaze-server/src/test/scala/org/http4s/blaze/server/BlazeServerMtlsSpec.scala

@@ -153,23 +153,26 @@ class BlazeServerMtlsSpec extends CatsEffectSuite {
     */
   blazeServer(TLSParameters(needClientAuth = true).toSSLParameters)
     .test("Server should send mTLS request correctly") { server =>
-      assertEquals(get(server, "/dummy", true), "CN=Test,OU=Test,O=Test,L=CA,ST=CA,C=US")
+      assertEquals(get(server, "/dummy"), "CN=Test,OU=Test,O=Test,L=CA,ST=CA,C=US")
     }
   blazeServer(TLSParameters(needClientAuth = true).toSSLParameters)
     .test("Server should fail for invalid client auth") { server =>
-      assertNotEquals(get(server, "/dummy", false), "CN=Test,OU=Test,O=Test,L=CA,ST=CA,C=US")
+      assertNotEquals(
+        get(server, "/dummy", clientAuth = false),
+        "CN=Test,OU=Test,O=Test,L=CA,ST=CA,C=US",
+      )
     }
 
   /** Test "requested" auth mode
     */
   blazeServer(TLSParameters(wantClientAuth = true).toSSLParameters)
     .test("Server should send mTLS request correctly with optional auth") { server =>
-      assertEquals(get(server, "/dummy", true), "CN=Test,OU=Test,O=Test,L=CA,ST=CA,C=US")
+      assertEquals(get(server, "/dummy"), "CN=Test,OU=Test,O=Test,L=CA,ST=CA,C=US")
     }
 
   blazeServer(TLSParameters(wantClientAuth = true).toSSLParameters)
     .test("Server should send mTLS request correctly without clientAuth") { server =>
-      assertEquals(get(server, "/noauth", false), "success")
+      assertEquals(get(server, "/noauth", clientAuth = false), "success")
     }
 
 }