Skip to content

Commit

Permalink
More cleanup after review feedback
Browse files Browse the repository at this point in the history
Added a semigroup instance (based on merge)
Adding a monoid is possible, but there are two zero cases. Changing the Path type to an ADT will resolve that.
  • Loading branch information
hamnis committed May 16, 2020
1 parent ec99d5c commit 77ecca1
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 33 deletions.
2 changes: 1 addition & 1 deletion core/src/main/scala/org/http4s/Message.scala
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ final class Request[F[_]](

def withPathInfo(pi: String): Self =
// Don't use withUri, which clears the caret
copy(uri = uri.withPath(scriptName + pi))
copy(uri = uri.withPath(Uri.Path.fromString(scriptName + pi)))

def pathTranslated: Option[File] = attributes.lookup(Keys.PathTranslated)

Expand Down
23 changes: 14 additions & 9 deletions core/src/main/scala/org/http4s/Uri.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import cats.{Eq, Hash, Order, Show}
import cats.syntax.either._
import java.net.{Inet4Address, Inet6Address, InetAddress}
import java.nio.{ByteBuffer, CharBuffer}
import java.nio.charset.{Charset => JCharset, StandardCharsets}
import org.http4s.internal.bug
import java.nio.charset.{StandardCharsets, Charset => JCharset}

import cats.kernel.Semigroup
import org.http4s.internal.bug
import org.http4s.internal.parboiled2.{Parser => PbParser, _}
import org.http4s.internal.parboiled2.CharPredicate.{Alpha, Digit, HexDigit}
import org.http4s.parser._
Expand Down Expand Up @@ -38,8 +39,9 @@ final case class Uri(
* Adds the path exactly as described. Any path element must be urlencoded ahead of time.
* @param path the path string to replace
*/
@deprecated("Use {withPath(Uri.Path)} instead", "1.0.0-M1")
def withPath(path: String): Uri = copy(path = Uri.Path.fromString(path))
//TODO: Maybe rename the string version

def withPath(path: Uri.Path): Uri = copy(path = path)

def withFragment(fragment: Uri.Fragment): Uri = copy(fragment = Option(fragment))
Expand Down Expand Up @@ -295,7 +297,7 @@ object Uri {
override def hashCode(): Int = {
var hash = segments.hashCode()
hash += 31 * java.lang.Boolean.hashCode(absolute)
hash += 31 * java.lang.Boolean.hashCode(absolute)
hash += 31 * java.lang.Boolean.hashCode(endsWithSlash)
hash
}

Expand Down Expand Up @@ -341,7 +343,7 @@ object Uri {
val empty = Path(Nil)
val Root = Path(Nil, absolute = true)

class Segment private (val encoded: String) {
final class Segment private (val encoded: String) {
def isEmpty = encoded.isEmpty

override def equals(obj: Any): Boolean = obj match {
Expand All @@ -350,6 +352,8 @@ object Uri {

override def hashCode(): Int = encoded.hashCode

def decoded: String = Uri.decode(encoded)

override val toString: String = encoded
}

Expand Down Expand Up @@ -393,6 +397,7 @@ object Uri {
}

implicit val eq: Eq[Path] = Eq.fromUniversalEquals[Path]
implicit val semigroup: Semigroup[Path] = (a: Path, b: Path) => a.merge(b)
}

/** The userinfo subcomponent may consist of a user name and,
Expand Down Expand Up @@ -776,21 +781,21 @@ object Uri {
else Uri(s, a, bp.merge(p), q, f)
}

target.withPath(removeDotSegments(target.path.renderString))
target.withPath(removeDotSegments(target.path))
}

/**
* Remove dot sequences from a Path, per RFC 3986 Sec 5.2.4
* Adapted from"
* https://github.com/Norconex/commons-lang/blob/c83fdeac7a60ac99c8602e0b47056ad77b08f570/norconex-commons-lang/src/main/java/com/norconex/commons/lang/url/URLNormalizer.java#L429
*/
def removeDotSegments(path: String): String = {
def removeDotSegments(path: Uri.Path): Uri.Path = {
// (Bulleted comments are from RFC3986, section-5.2.4)

// 1. The input buffer is initialized with the now-appended path
// components and the output buffer is initialized to the empty
// string.
val in = new StringBuilder(path)
val in = new StringBuilder(path.renderString)
val out = new StringBuilder

// 2. While the input buffer is not empty, loop as follows:
Expand Down Expand Up @@ -848,7 +853,7 @@ object Uri {

// 3. Finally, the output buffer is returned as the result of
// remove_dot_segments.
out.toString
Uri.Path.fromString(out.toString)
}

// Helper functions for removeDotSegments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.http4s.circe._
import org.http4s.client.Client
import org.http4s.client.dsl.Http4sClientDsl
import org.http4s.{Header, Request, Uri}
import org.http4s.syntax.literals._

// See: https://developer.github.com/apps/building-oauth-apps/authorization-options-for-oauth-apps/#web-application-flow
class GitHubService[F[_]: Sync](client: Client[F]) extends Http4sClientDsl[F] {
Expand All @@ -23,7 +24,7 @@ class GitHubService[F[_]: Sync](client: Client[F]) extends Http4sClientDsl[F] {
val authorize: Stream[F, Byte] = {
val uri = Uri
.uri("https://github.com")
.withPath("/login/oauth/authorize")
.withPath(path"/login/oauth/authorize")
.withQueryParam("client_id", ClientId)
.withQueryParam("redirect_uri", RedirectUri)
.withQueryParam("scopes", "public_repo")
Expand All @@ -33,9 +34,8 @@ class GitHubService[F[_]: Sync](client: Client[F]) extends Http4sClientDsl[F] {
}

def accessToken(code: String, state: String): F[String] = {
val uri = Uri
.uri("https://github.com")
.withPath("/login/oauth/access_token")
val uri = uri"https://github.com"
.withPath(path"/login/oauth/access_token")
.withQueryParam("client_id", ClientId)
.withQueryParam("client_secret", ClientSecret)
.withQueryParam("code", code)
Expand All @@ -48,7 +48,7 @@ class GitHubService[F[_]: Sync](client: Client[F]) extends Http4sClientDsl[F] {
}

def userData(accessToken: String): F[String] = {
val request = Request[F](uri = Uri.uri("https://api.github.com/user"))
val request = Request[F](uri = uri"https://api.github.com/user")
.putHeaders(Header("Authorization", s"token $accessToken"))

client.expect[String](request)
Expand Down
2 changes: 1 addition & 1 deletion tests/src/test/scala/org/http4s/MessageSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class MessageSpec extends Http4sSpec with Http4sLegacyMatchersIO {

"reset pathInfo if uri is changed" in {
val originalReq = Request(uri = path1, attributes = attributes)
val updatedReq = originalReq.withUri(uri = Uri().withPath(path2))
val updatedReq = originalReq.withUri(uri = Uri().withPath(Uri.Path.fromString(path2)))

updatedReq.scriptName mustEqual ""
updatedReq.pathInfo mustEqual path2
Expand Down
24 changes: 12 additions & 12 deletions tests/src/test/scala/org/http4s/UriSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ http://example.org/a file

"withPath without slash adds a / on render" in {
val uri = getUri("http://localhost/foo/bar/baz")
uri.withPath("bar").toString must_=== "http://localhost/bar"
uri.withPath(path"bar").toString must_=== "http://localhost/bar"
}

"render a IPv6 address, should be wrapped in brackets" in {
Expand Down Expand Up @@ -809,14 +809,14 @@ http://example.org/a file
val base = getUri("http://a/b/c/d;p?q")

"correctly remove ./.. sequences" >> {
implicit class checkDotSequences(path: String) {
def removingDotsShould_==(expected: String) =
implicit class checkDotSequences(path: Uri.Path) {
def removingDotsShould_==(expected: Uri.Path) =
s"$path -> $expected" in { removeDotSegments(path) must_== expected }
}

// from RFC 3986 sec 5.2.4
"mid/content=5/../6".removingDotsShould_==("mid/6")
"/a/b/c/./../../g".removingDotsShould_==("/a/g")
path"mid/content=5/../6".removingDotsShould_==(path"mid/6")
path"/a/b/c/./../../g".removingDotsShould_==(path"/a/g")
}

implicit class check(relative: String) {
Expand Down Expand Up @@ -891,7 +891,7 @@ http://example.org/a file

"correctly remove dot segments in other examples" >> prop { (input: String) =>
val prefix = "/this/isa/prefix/"
val processed = Uri.removeDotSegments(input)
val processed = Uri.removeDotSegments(Uri.Path.fromString(input)).renderString
val path = Paths.get(prefix, processed).normalize
path.startsWith(Paths.get(prefix)) must beTrue
processed must not contain "./"
Expand All @@ -909,7 +909,7 @@ http://example.org/a file

"Uri.equals" should {
"be false between an empty path and a trailing slash after an authority" in {
uri("http://example.com") must_!= uri("http://example.com/")
getUri("http://example.com") must_!= getUri("http://example.com/")
}
}

Expand All @@ -921,24 +921,24 @@ http://example.org/a file

"/" should {
"encode space as %20" in {
uri("http://example.com/") / " " must_== uri("http://example.com/%20")
getUri("http://example.com/") / " " must_== getUri("http://example.com/%20")
}

"encode generic delimiters that aren't pchars" in {
// ":" and "@" are valid pchars
uri("http://example.com") / ":/?#[]@" must_== uri("http://example.com/:%2F%3F%23%5B%5D@")
getUri("http://example.com") / ":/?#[]@" must_== getUri("http://example.com/:%2F%3F%23%5B%5D@")
}

"encode percent sequences" in {
uri("http://example.com") / "%2F" must_== uri("http://example.com/%252F")
getUri("http://example.com") / "%2F" must_== getUri("http://example.com/%252F")
}

"not encode sub-delims" in {
uri("http://example.com") / "!$&'()*+,;=" must_== uri("http://example.com/!$&'()*+,;=")
getUri("http://example.com") / "!$&'()*+,;=" must_== getUri("http://example.com/!$&'()*+,;=")
}

"UTF-8 encode characters" in {
uri("http://example.com/") / "ö" must_== uri("http://example.com/%C3%B6")
getUri("http://example.com/") / "ö" must_== getUri("http://example.com/%C3%B6")
}

"not make bad URIs" >> forAll { (s: String) =>
Expand Down
7 changes: 2 additions & 5 deletions tests/src/test/scala/org/http4s/multipart/MultipartSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,14 @@ import cats.implicits._
import fs2._
import java.io.File
import org.http4s.headers._
import org.http4s.Uri._
import org.http4s.syntax.literals._
import org.http4s.EntityEncoder._
import org.specs2.mutable.Specification

class MultipartSpec extends Specification {
implicit val contextShift: ContextShift[IO] = IO.contextShift(Http4sSpec.TestExecutionContext)

val url = Uri(
scheme = Some(Scheme.https),
authority = Some(Authority(host = RegName("example.com")))
).withPath("/path/to/some/where")
val url = uri"https://example.com/path/to/some/where"

implicit def partIOEq: Eq[Part[IO]] = Eq.instance[Part[IO]] {
case (a, b) =>
Expand Down

0 comments on commit 77ecca1

Please sign in to comment.