Repository: scala-steward-org/scala-steward (scala)

Issue:
Scala Steward generating invalid release links
Hi, thanks again for Scala Steward.

I've hit a new bug with our installation/operating of Scala Steward. 

**Information**

- Version: Scala Steward v0.12.0
- VCS: private GitLab instance

**The problem**

My understanding is that when you configure `homepage` in your sbt build, it ends up in the pom. And when Scala Steward reads that pom it grabs the URL to generate the `ReleaseRelatedUrl`. When using a private GitLab instance, or atleast in the way it's configured for us, trying to do a `HEAD` to a file (eg: "https://internal.gitlab.company.com/tools/scala/some-project/blob/master/ReleaseNotes.md") will give you a 302 with a `Location: https://internal.gitlab.company.com/users/sign_in`.

Because the `OkHttpClient` in use by the `UrlChecker` follows redirect, this ends up with a `200 OK` and thus the file url is considered valid when, in reality, it most likely is not.

eg:

![Screen Shot 2021-11-17 at 12 12 17](https://user-images.githubusercontent.com/5230460/142248733-32c7973e-6901-4238-9ec5-070c4668f8a7.png)

**The possible solutions:**

1. disable redirect for the http client (I don't know if that's possible, especially when you consider the behaviour of GitHub when renaming a repo (redirects to the new one, eg: `fthomas/scala-steward` now points to this repository.))
2. have an alternative client with redirect disabled only for url checking
3. use the gitlab api (authenticated) to check for files existence (https://docs.gitlab.com/ee/api/repository_files.html)

Personally, I would consider the second, and if that does not work the third one.

What do you think?

Thanks

Pull request:
this addresses #2363

Patch:
diff --git a/build.sbt b/build.sbt
index 6bf1e1825d..22493bde39 100644
--- a/build.sbt
+++ b/build.sbt
@@ -114,6 +114,7 @@ lazy val core = myCrossProject("core")
       Dependencies.circeLiteral % Test,
       Dependencies.disciplineMunit % Test,
       Dependencies.http4sDsl % Test,
+      Dependencies.http4sBlazeServer % Test,
       Dependencies.munit % Test,
       Dependencies.munitCatsEffect % Test,
       Dependencies.munitScalacheck % Test,
diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
index 679da332e8..10db8ed340 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
@@ -22,11 +22,11 @@ import cats.syntax.all._
 import org.http4s.Uri
 import org.http4s.client.Client
 import org.http4s.headers.`User-Agent`
-import org.http4s.okhttp.client.OkHttpBuilder
 import org.scalasteward.core.buildtool.BuildToolDispatcher
 import org.scalasteward.core.buildtool.maven.MavenAlg
 import org.scalasteward.core.buildtool.mill.MillAlg
 import org.scalasteward.core.buildtool.sbt.SbtAlg
+import org.scalasteward.core.client.ClientConfiguration
 import org.scalasteward.core.coursier.{CoursierAlg, VersionsCache}
 import org.scalasteward.core.edit.EditAlg
 import org.scalasteward.core.edit.hooks.HookExecutor
@@ -83,18 +83,34 @@ object Context {
       config = Config.from(args)
       _ <- Resource.eval(F.delay(System.setProperty("http.agent", userAgentString)))
       userAgent <- Resource.eval(F.fromEither(`User-Agent`.parse(userAgentString)))
-      client <- OkHttpBuilder
-        .withDefaultClient[F]
-        .flatMap(_.resource)
-        .map(c => Client[F](req => c.run(req.putHeaders(userAgent))))
+      userAgentMiddleware = ClientConfiguration.setUserAgent[F](userAgent)
+      defaultClient <- ClientConfiguration.build(
+        ClientConfiguration.BuilderMiddleware.default,
+        userAgentMiddleware
+      )
+      urlCheckerClient <- ClientConfiguration.build(
+        ClientConfiguration.disableFollowRedirect[F],
+        userAgentMiddleware
+      )
       fileAlg = FileAlg.create(logger, F)
       processAlg = ProcessAlg.create(config.processCfg)(logger, F)
       workspaceAlg = WorkspaceAlg.create(config)(fileAlg, logger, F)
-      context <- Resource.eval(step1(config)(client, fileAlg, logger, processAlg, workspaceAlg, F))
+      context <- Resource.eval(
+        step1(config)(
+          defaultClient,
+          UrlCheckerClient(urlCheckerClient),
+          fileAlg,
+          logger,
+          processAlg,
+          workspaceAlg,
+          F
+        )
+      )
     } yield context
 
   def step1[F[_]](config: Config)(implicit
       client: Client[F],
+      urlCheckerClient: UrlCheckerClient[F],
       fileAlg: FileAlg[F],
       logger: Logger[F],
       processAlg: ProcessAlg[F],
diff --git a/modules/core/src/main/scala/org/scalasteward/core/client/ClientConfiguration.scala b/modules/core/src/main/scala/org/scalasteward/core/client/ClientConfiguration.scala
new file mode 100644
index 0000000000..38a4c6371d
--- /dev/null
+++ b/modules/core/src/main/scala/org/scalasteward/core/client/ClientConfiguration.scala
@@ -0,0 +1,44 @@
+/*
+ * Copyright 2018-2021 Scala Steward contributors
+ *
+ * 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.scalasteward.core.client
+
+import org.http4s.client._
+import org.http4s.headers.`User-Agent`
+import cats.effect._
+import org.http4s.okhttp.client.OkHttpBuilder
+
+object ClientConfiguration {
+  type BuilderMiddleware[F[_]] = OkHttpBuilder[F] => OkHttpBuilder[F]
+  object BuilderMiddleware {
+    def default[F[_]]: BuilderMiddleware[F] = bmw => bmw
+  }
+  def build[F[_]: Async](bmw: BuilderMiddleware[F], cmw: Middleware[F]): Resource[F, Client[F]] =
+    OkHttpBuilder
+      .withDefaultClient[F]
+      .map(bmw)
+      .flatMap(_.resource)
+      .map(cmw)
+
+  def setUserAgent[F[_]: MonadCancelThrow](userAgent: `User-Agent`): Middleware[F] = { client =>
+    Client[F](req => client.run(req.putHeaders(userAgent)))
+  }
+
+  def disableFollowRedirect[F[_]](builder: OkHttpBuilder[F]): OkHttpBuilder[F] =
+    builder.withOkHttpClient(
+      builder.okHttpClient.newBuilder().followRedirects(false).build()
+    )
+}
diff --git a/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala b/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala
index 7c0d415e0d..440e182dc1 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala
@@ -30,6 +30,8 @@ trait UrlChecker[F[_]] {
   def exists(url: Uri): F[Boolean]
 }
 
+final case class UrlCheckerClient[F[_]](client: Client[F]) extends AnyVal
+
 object UrlChecker {
   private def buildCache[F[_]](config: Config)(implicit F: Sync[F]): F[CaffeineCache[F, Status]] =
     F.delay {
@@ -42,7 +44,7 @@ object UrlChecker {
     }
 
   def create[F[_]](config: Config)(implicit
-      client: Client[F],
+      urlCheckerClient: UrlCheckerClient[F],
       logger: Logger[F],
       F: Sync[F]
   ): F[UrlChecker[F]] =
@@ -55,7 +57,7 @@ object UrlChecker {
 
         private def status(url: Uri): F[Status] =
           statusCache.cachingForMemoizeF(url.renderString)(None) {
-            client.status(Request[F](method = Method.HEAD, uri = url))
+            urlCheckerClient.client.status(Request[F](method = Method.HEAD, uri = url))
           }
       }
     }
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index 79b379d0d1..a8408f51bf 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -26,6 +26,7 @@ object Dependencies {
   val http4sCirce = "org.http4s" %% "http4s-circe" % http4sCore.revision
   val http4sClient = "org.http4s" %% "http4s-client" % http4sCore.revision
   val http4sDsl = "org.http4s" %% "http4s-dsl" % http4sCore.revision
+  val http4sBlazeServer = "org.http4s" %% "http4s-blaze-server" % http4sCore.revision
   val http4sOkhttpClient = "org.http4s" %% "http4s-okhttp-client" % http4sCore.revision
   val log4catsSlf4j = "org.typelevel" %% "log4cats-slf4j" % "2.1.1"
   val logbackClassic = "ch.qos.logback" % "logback-classic" % "1.2.7"


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala b/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala
new file mode 100644
index 0000000000..8fc0df36fe
--- /dev/null
+++ b/modules/core/src/test/scala/org/scalasteward/core/client/ClientConfigurationTest.scala
@@ -0,0 +1,86 @@
+package org.scalasteward.core.client
+
+import org.http4s.client._
+import org.http4s.headers.`User-Agent`
+import cats.effect._
+import cats.implicits._
+import org.http4s.implicits._
+import org.typelevel.ci._
+import munit.CatsEffectSuite
+import org.http4s.HttpRoutes
+import org.http4s.headers.Location
+
+class ClientConfigurationTest extends CatsEffectSuite {
+  private val userAgentValue = "my-user-agent"
+  private val dummyUserAgent =
+    `User-Agent`.parse(userAgentValue).getOrElse(fail("unable to create user agent"))
+
+  private val routes: HttpRoutes[IO] = {
+    import org.http4s.dsl.io._
+    HttpRoutes.of[IO] {
+      case req @ GET -> Root / "user-agent" =>
+        req.headers.get(ci"user-agent") match {
+          case Some(value) =>
+            if (value.head.value == userAgentValue) Ok("success")
+            else
+              IO.raiseError(
+                new RuntimeException(s"expected $userAgentValue but got ${value.head.value}")
+              )
+          case None =>
+            BadRequest("No user-agent")
+        }
+
+      case GET -> Root / "redirect" =>
+        Found(Location(uri"/redirected"))
+
+      case GET -> Root / "redirected" =>
+        BadRequest("Got redirected")
+    }
+  }
+
+  test("setUserAgent add a specific user agent to requests") {
+    import org.http4s.client.dsl.io._
+    import org.http4s.Method._
+
+    val initialClient = Client.fromHttpApp[IO](routes.orNotFound)
+    val setUserAgent = ClientConfiguration.setUserAgent[IO](dummyUserAgent)
+    val newClient = setUserAgent(initialClient)
+
+    newClient
+      .run(GET(uri"/user-agent"))
+      .use(r => r.status.code.pure[IO])
+      .assertEquals(200)
+
+    initialClient
+      .run(GET(uri"/user-agent"))
+      .use(r => r.status.code.pure[IO])
+      .assertEquals(400)
+  }
+
+  test("disableFollowRedirect does not follow redirect") {
+    import org.http4s.blaze.server._
+    import org.http4s.client.dsl.io._
+    import org.http4s.Method._
+
+    val regularClient = ClientConfiguration.build[IO](
+      ClientConfiguration.BuilderMiddleware.default,
+      ClientConfiguration.setUserAgent(dummyUserAgent)
+    )
+    val disabledClient = ClientConfiguration.build[IO](
+      ClientConfiguration.disableFollowRedirect,
+      ClientConfiguration.setUserAgent(dummyUserAgent)
+    )
+    val getServer =
+      BlazeServerBuilder[IO].bindAny("localhost").withHttpApp(routes.orNotFound).resource
+
+    val test = (regularClient, disabledClient, getServer).tupled.use {
+      case (regClient, disClient, s) =>
+        val fullUri = s.baseUri / "redirect"
+        (
+          regClient.run(GET(fullUri)).use(_.status.code.pure[IO]),
+          disClient.run(GET(fullUri)).use(_.status.code.pure[IO])
+        ).tupled
+    }
+    test.assertEquals((400, 302))
+  }
+}
diff --git a/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala b/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
index 4001b399a0..09740e8a8a 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
@@ -10,10 +10,12 @@ import org.scalasteward.core.io._
 import org.scalasteward.core.mock.MockConfig.config
 import org.scalasteward.core.repoconfig.RepoConfigLoader
 import org.scalasteward.core.update.artifact.ArtifactMigrationsLoader
+import org.scalasteward.core.util.UrlCheckerClient
 import org.typelevel.log4cats.Logger
 
 object MockContext {
   implicit private val client: Client[MockEff] = Client.fromHttpApp(HttpApp.notFound)
+  implicit private val urlCheckerClient: UrlCheckerClient[MockEff] = UrlCheckerClient(client)
   implicit private val fileAlg: FileAlg[MockEff] = new MockFileAlg
   implicit private val logger: Logger[MockEff] = new MockLogger
   implicit private val processAlg: ProcessAlg[MockEff] = MockProcessAlg.create(config.processCfg)
diff --git a/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSExtraAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSExtraAlgTest.scala
index e5a1020bee..4818c313e3 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSExtraAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSExtraAlgTest.scala
@@ -12,7 +12,7 @@ import org.scalasteward.core.TestSyntax._
 import org.scalasteward.core.application.Config.VCSCfg
 import org.scalasteward.core.data.ReleaseRelatedUrl
 import org.scalasteward.core.mock.MockConfig
-import org.scalasteward.core.util.UrlChecker
+import org.scalasteward.core.util._
 
 class VCSExtraAlgTest extends FunSuite {
   val routes: HttpRoutes[IO] =
@@ -22,7 +22,8 @@ class VCSExtraAlgTest extends FunSuite {
       case _                                                            => NotFound()
     }
 
-  implicit val client: Client[IO] = Client.fromHttpApp[IO](routes.orNotFound)
+  implicit val client: UrlCheckerClient[IO] =
+    UrlCheckerClient[IO](Client.fromHttpApp[IO](routes.orNotFound))
   implicit val urlChecker: UrlChecker[IO] =
     UrlChecker.create[IO](MockConfig.config).unsafeRunSync()