From 2e8b4869e54be879102586a1a5c9d340d0829c5c Mon Sep 17 00:00:00 2001 From: Jacob Gulotta Date: Fri, 8 Dec 2023 03:31:24 -0800 Subject: [PATCH] Support URL relative resolution, and use it in a ZClientAspect to follow redirects (#2537) * RFC3986 Relative Resolution * Add follow redirects client aspect --- zio-http/src/main/scala/zio/http/Path.scala | 54 +++++++- zio-http/src/main/scala/zio/http/URL.scala | 80 +++++++++++- .../main/scala/zio/http/ZClientAspect.scala | 120 +++++++++++++++++- .../src/test/scala/zio/http/PathSpec.scala | 51 ++++++++ .../src/test/scala/zio/http/URLSpec.scala | 82 ++++++++++++ .../scala/zio/http/ZClientAspectSpec.scala | 22 +++- 6 files changed, 404 insertions(+), 5 deletions(-) diff --git a/zio-http/src/main/scala/zio/http/Path.scala b/zio-http/src/main/scala/zio/http/Path.scala index 81c7f4f68a..c859a6b61c 100644 --- a/zio-http/src/main/scala/zio/http/Path.scala +++ b/zio-http/src/main/scala/zio/http/Path.scala @@ -16,8 +16,6 @@ package zio.http -import scala.collection.mutable - import zio.{Chunk, ChunkBuilder} /** @@ -184,6 +182,58 @@ final case class Path private[http] (flags: Path.Flags, segments: Chunk[String]) else Path.empty } else self + /** + * RFC 3986 § 5.2.4 Remove Dot Segments + * @return + * the Path with `.` and `..` resolved and removed + */ + def removeDotSegments: Path = { + // See https://www.rfc-editor.org/rfc/rfc3986#section-5.2.4 + val segments = new Array[String](self.segments.length) + var segmentCount = 0 + // leading/trailing slashes may change but is unlikely + var flags = self.flags + + var i = 0 + val max = self.segments.length + + if (!Flag.LeadingSlash.check(flags)) { + // § 5.2.4.2.A/D no leading slash, so skip all initial `./` and `../` + while (i < max && (self.segments(i) == "." | self.segments(i) == "..")) { + i += 1 + } + // if the entire input was consumed, there is no more trailing slash + if (i == max) flags = Flag.TrailingSlash.remove(flags) + } + + var loop = i < max + while (loop) { + val segment = self.segments(i) + + i += 1 + loop = i < max + + if (segment == "..") { + segmentCount = (segmentCount - 1).max(0) + // § 5.2.4.2.C resolving `/..` and `/../` removes preceding slashes and is itself replaced by a slash + // so if we popped the first one we definitely have a leading slash + if (segmentCount == 0) flags = Flag.LeadingSlash.add(flags) + // § 5.2.4.2.C resolving `/..` and `/../` are both as-if replaced by a `/` + // so if this is the last segment, then we have a trailing slash + if (i == max) flags = Flag.TrailingSlash.add(flags) + } else if (segment == ".") { + // § 5.2.4.2.B resolving `/.` and `/./` are both as-if replaced by a `/` + // so if this is the last segment, then we have a trailing slash + if (i == max) flags = Flag.TrailingSlash.add(flags) + } else { + segments(segmentCount) = segment + segmentCount += 1 + } + } + + Path(flags, Chunk.fromArray(segments.take(segmentCount))) + } + /** * Creates a new path from this one with it's segments reversed. */ diff --git a/zio-http/src/main/scala/zio/http/URL.scala b/zio-http/src/main/scala/zio/http/URL.scala index 6ae6507487..281f842e1d 100644 --- a/zio-http/src/main/scala/zio/http/URL.scala +++ b/zio-http/src/main/scala/zio/http/URL.scala @@ -16,7 +16,7 @@ package zio.http -import java.net.{MalformedURLException, URI, URISyntaxException} +import java.net.{MalformedURLException, URI} import scala.util.Try @@ -184,6 +184,84 @@ final case class URL( case _ => self.copy(kind = URL.Location.Relative) } + /** + * RFC 3986 § 5.2 Relative Resolution + * @param reference + * the URL to resolve relative to ``this`` base URL + * @return + * the target URL + */ + def resolve(reference: URL): Either[String, URL] = { + // See https://www.rfc-editor.org/rfc/rfc3986#section-5.2 + // § 5.2.1 - `self` is the base and already pre-parsed into components + // § 5.2.2 - strict parsing does not ignore the reference URL scheme, so we use it directly, instead of un-setting it + + if (reference.kind.isRelative) { + // § 5.2.2 - reference scheme is undefined, i.e. it is relative + self.kind match { + // § 5.2.1 - `self` is the base and is required to have a scheme, therefore it must be absolute + case Location.Relative => Left("cannot resolve against relative url") + + case location: Location.Absolute => + var path: Path = null + var query: QueryParams = null + + if (reference.path.isEmpty) { + // § 5.2.2 - empty reference path keeps base path unmodified + path = self.path + // § 5.2.2 - given an empty reference path, use non-empty reference query params, + // while empty reference query params keeps base query params + // NOTE: strictly, if the reference defines a query it should be used, even if that query is empty + // but currently no-query is not differentiated from empty-query + if (reference.queryParams.isEmpty) { + query = self.queryParams + } else { + query = reference.queryParams + } + } else { + // § 5.2.2 - non-empty reference path always keeps reference query params + query = reference.queryParams + + if (reference.path.hasLeadingSlash) { + // § 5.2.2 - reference path starts from root, keep reference path without dot segments + path = reference.path.removeDotSegments + } else { + // § 5.2.2 - merge base and reference paths, then collapse dot segments + // § 5.2.3 - if base has an authority AND an empty path, use the reference path, ensuring a leading slash + // the authority is the [user]@host[:port], which is always present on `self`, + // so we only need to check for an empty path + if (self.path.isEmpty) { + path = reference.path.addLeadingSlash + } else { + // § 5.2.3 - otherwise (base has no authority OR a non-empty path), drop the very last portion of the base path, + // and append all the reference path components + path = Path( + Path.Flags.concat(self.path.flags, reference.path.flags), + self.path.segments.dropRight(1) ++ reference.path.segments, + ) + } + + path = path.removeDotSegments + } + } + + val url = URL(path, location, query, reference.fragment) + + Right(url) + + } + } else { + // § 5.2.2 - if the reference scheme is defined, i.e. the reference is absolute, + // the target components are the reference components but with dot segments removed + + // § 5.2.2 - if the reference scheme is undefined and authority is defined, keep the base scheme + // and take everything else from the reference, removing dot segments from the path + // NOTE: URL currently does not track authority separate from scheme to implement this + // so having an authority is the same as having a scheme and they are treated the same + Right(reference.copy(path = reference.path.removeDotSegments)) + } + } + def scheme: Option[Scheme] = kind match { case Location.Absolute(scheme, _, _) => Some(scheme) case Location.Relative => None diff --git a/zio-http/src/main/scala/zio/http/ZClientAspect.scala b/zio-http/src/main/scala/zio/http/ZClientAspect.scala index 82bf529893..bf7c011f2e 100644 --- a/zio-http/src/main/scala/zio/http/ZClientAspect.scala +++ b/zio-http/src/main/scala/zio/http/ZClientAspect.scala @@ -222,11 +222,12 @@ object ZClientAspect { case (duration, Exit.Success(response)) => ZIO .logLevel(level(response.status)) { - def requestHeaders = + def requestHeaders = headers.collect { case header: Header if loggedRequestHeaderNames.contains(header.headerName.toLowerCase) => LogAnnotation(header.headerName, header.renderedValue) }.toSet + def responseHeaders = response.headers.collect { case header: Header if loggedResponseHeaderNames.contains(header.headerName.toLowerCase) => @@ -318,4 +319,121 @@ object ZClientAspect { } } } + + final def followRedirects[R, E](max: Int)( + onRedirectError: (Response, String) => ZIO[R, E, Response], + )(implicit trace: Trace): ZClientAspect[Nothing, R, Nothing, Body, E, Any, Nothing, Response] = { + new ZClientAspect[Nothing, R, Nothing, Body, E, Any, Nothing, Response] { + override def apply[ + Env >: Nothing <: R, + In >: Nothing <: Body, + Err >: E <: Any, + Out >: Nothing <: Response, + ](client: ZClient[Env, In, Err, Out]): ZClient[Env, In, Err, Out] = { + val oldDriver = client.driver + + val newDriver = new ZClient.Driver[Env, Err] { + def scopedRedirectErr(resp: Response, message: String) = + ZIO.scopeWith(_ => onRedirectError(resp, message)) + + override def request( + version: Version, + method: Method, + url: URL, + headers: Headers, + body: Body, + sslConfig: Option[ClientSSLConfig], + proxy: Option[Proxy], + )(implicit trace: Trace): ZIO[Env & Scope, Err, Response] = { + def req( + attempt: Int, + version: Version, + method: Method, + url: URL, + headers: Headers, + body: Body, + sslConfig: Option[ClientSSLConfig], + proxy: Option[Proxy], + ): ZIO[Env & Scope, Err, Response] = { + oldDriver.request(version, method, url, headers, body, sslConfig, proxy).flatMap { resp => + if (resp.status.isRedirection) { + if (attempt < max) { + resp.headerOrFail(Header.Location) match { + case Some(locOrError) => + locOrError match { + case Left(locHeaderErr) => + scopedRedirectErr(resp, locHeaderErr) + + case Right(loc) => + url.resolve(loc.url) match { + case Left(relativeResolveErr) => + scopedRedirectErr(resp, relativeResolveErr) + + case Right(resolved) => + req(attempt + 1, version, method, resolved, headers, body, sslConfig, proxy) + } + } + case None => + scopedRedirectErr(resp, "no location header to resolve redirect") + } + } else { + scopedRedirectErr(resp, "followed maximum redirects") + } + } else { + ZIO.succeed(resp) + } + } + } + + req(0, version, method, url, headers, body, sslConfig, proxy) + } + + override def socket[Env1 <: Env](version: Version, url: URL, headers: Headers, app: WebSocketApp[Env1])( + implicit trace: Trace, + ): ZIO[Env1 & Scope, Err, Response] = { + def sock( + attempt: Int, + version: Version, + url: URL, + headers: Headers, + app: WebSocketApp[Env1], + ): ZIO[Env1 & Scope, Err, Response] = { + oldDriver.socket(version, url, headers, app).flatMap { resp => + if (resp.status.isRedirection) { + if (attempt < max) { + resp.headerOrFail(Header.Location) match { + case Some(locOrError) => + locOrError match { + case Left(locHeaderErr) => + scopedRedirectErr(resp, locHeaderErr) + + case Right(loc) => + url.resolve(loc.url) match { + case Left(relativeResolveErr) => + scopedRedirectErr(resp, relativeResolveErr) + + case Right(resolved) => + sock(attempt + 1, version, resolved, headers, app) + } + } + case None => + scopedRedirectErr(resp, "no location header to resolve redirect") + } + } else { + scopedRedirectErr(resp, "followed maximum redirects") + } + } else { + ZIO.succeed(resp) + } + } + } + + sock(0, version, url, headers, app) + } + } + + client.transform(client.bodyEncoder, client.bodyDecoder, newDriver) + } + } + } } diff --git a/zio-http/src/test/scala/zio/http/PathSpec.scala b/zio-http/src/test/scala/zio/http/PathSpec.scala index 6c0b0dbb96..c3dc1a2338 100644 --- a/zio-http/src/test/scala/zio/http/PathSpec.scala +++ b/zio-http/src/test/scala/zio/http/PathSpec.scala @@ -413,5 +413,56 @@ object PathSpec extends ZIOHttpSpec with ExitAssertion { } }, ), + suite("removeDotSegments")( + test("only leading slash and dots") { + val path = Path.decode("/./../") + val result = path.removeDotSegments + val expected = Path.root + + assertTrue(result == expected) + }, + test("only leading dots") { + val path = Path.decode("./../") + val result = path.removeDotSegments + val expected = Path.empty + + assertTrue(result == expected) + }, + test("leading slash and dots") { + val path = Path.decode("/./../path") + val result = path.removeDotSegments + val expected = Path.decode("/path") + + assertTrue(result == expected) + }, + test("leading dots and path") { + val path = Path.decode("./../path") + val result = path.removeDotSegments + val expected = Path.decode("path") + + assertTrue(result == expected) + }, + test("double dot to top") { + val path = Path.decode("path/../subpath") + val result = path.removeDotSegments + val expected = Path.decode("/subpath") + + assertTrue(result == expected) + }, + test("trailing double dots") { + val path = Path.decode("path/ignored/..") + val result = path.removeDotSegments + val expected = Path.decode("path/") + + assertTrue(result == expected) + }, + test("path traversal") { + val path = Path.decode("/start/ignored/./../path/also/ignored/../../end/.") + val result = path.removeDotSegments + val expected = Path.decode("/start/path/end/") + + assertTrue(result == expected) + }, + ), ) } diff --git a/zio-http/src/test/scala/zio/http/URLSpec.scala b/zio-http/src/test/scala/zio/http/URLSpec.scala index e6521f90be..4d3c7f2280 100644 --- a/zio-http/src/test/scala/zio/http/URLSpec.scala +++ b/zio-http/src/test/scala/zio/http/URLSpec.scala @@ -239,5 +239,87 @@ object URLSpec extends ZIOHttpSpec { assertZIO(result)(isLeft) }, ), + suite("relative resolution")( + // next ones are edge cases + test("absolute reference with relative base") { + val base = url"base/relative#basefrag" + val reference = url"https://reference/ignored/.././absolute#reffrag" + + // uses reference without dot segments + val expected = url"https://reference/absolute#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + test("absolute reference with absolute base") { + val base = url"https://base#basefrag" + val reference = url"https://reference/ignored/.././absolute#reffrag" + + // uses reference without dot segments + val expected = url"https://reference/absolute#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + test("relative reference with relative base") { + val base = url"base/relative" + val reference = url"reference/relative" + + val result = base.resolve(reference) + assertTrue(result.isLeft) + }, + + // remainder are main resolution logic - absolute base, relative reference + test("empty reference path without query params") { + val base = url"https://base/./ignored/../absolute?param=base#basefrag" + val reference = url"#reffrag" + + // uses unmodified base path and base query params + val expected = url"https://base/./ignored/../absolute?param=base#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + test("empty reference path with query params") { + val base = url"https://base/./ignored/../absolute?param=base#basefrag" + val reference = url"?param=reference#reffrag" + + // uses unmodified base path and reference query params + val expected = url"https://base/./ignored/../absolute?param=reference#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + test("non-empty reference path with a leading slash") { + val base = url"https://base/./ignored/../first/second?param=base#basefrag" + val reference = url"/reference/./ignored/../last?param=reference#reffrag" + + // uses reference path without dot segments and reference query params + val expected = url"https://base/reference/last?param=reference#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + test("non-empty reference path without a leading slash") { + val base = url"https://base/./ignored/../first/..?param=base#basefrag" + val reference = url"reference/./ignored/../last?param=reference#reffrag" + + // uses base path without last segment, reference segments appended, without dot segments, and reference query params + val expected = url"https://base/first/reference/last?param=reference#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + test("non-empty reference path without a leading slash and empty base path") { + val base = url"https://base?param=base#basefrag" + val reference = url"reference/./ignored/../last?param=reference#reffrag" + + // uses reference path without dot segments and a leading slash + val expected = url"https://base/reference/last?param=reference#reffrag" + + val result = base.resolve(reference) + assertTrue(result.contains(expected)) + }, + ), ) } diff --git a/zio-http/src/test/scala/zio/http/ZClientAspectSpec.scala b/zio-http/src/test/scala/zio/http/ZClientAspectSpec.scala index beaa59edfa..394e92b461 100644 --- a/zio-http/src/test/scala/zio/http/ZClientAspectSpec.scala +++ b/zio-http/src/test/scala/zio/http/ZClientAspectSpec.scala @@ -25,7 +25,13 @@ import zio.http.URL.Location object ZClientAspectSpec extends ZIOHttpSpec { def extractStatus(response: Response): Status = response.status - val app: HttpApp[Any] = Handler.fromFunction[Request] { _ => Response.text("hello") }.toHttpApp + val app: HttpApp[Any] = { + Route.handled(Method.GET / "hello")(Handler.response(Response.text("hello"))) + }.toHttpApp + + val redir: HttpApp[Any] = { + Route.handled(Method.GET / "redirect")(Handler.response(Response.redirect(URL.empty / "hello"))) + }.toHttpApp override def spec: Spec[TestEnvironment with Scope, Any] = suite("ZClientAspect")( @@ -78,6 +84,20 @@ object ZClientAspectSpec extends ZIOHttpSpec { annotations.head.contains("duration_ms"), ), ), + test("followRedirects")( + for { + port <- Server.install(redir ++ app) + baseClient <- ZIO.service[Client] + client = baseClient + .url( + URL(Path.empty, Location.Absolute(Scheme.HTTP, "localhost", Some(port))), + ) + .disableStreaming @@ ZClientAspect.followRedirects(2)((resp, message) => ZIO.logInfo(message).as(resp)) + response <- client.request(Request.get(URL.empty / "redirect")) + } yield assertTrue( + extractStatus(response) == Status.Ok, + ), + ), ).provide( ZLayer.succeed(Server.Config.default.onAnyOpenPort), Server.live,