Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better default host/port/protocol when running in browser #81

Open
lj-ditrapani opened this issue Jun 23, 2018 · 3 comments
Open

Better default host/port/protocol when running in browser #81

lj-ditrapani opened this issue Jun 23, 2018 · 3 comments

Comments

@lj-ditrapani
Copy link

Hello,

I like this library, thanks for creating it!

When running in the browser, would it be reasonable to provide default HttpRequest parameter values based off of document.location if the user does not specify host, port and/or protocol? For example, I find myself writing this code whenever I use roshttp on the client side to make requests back the originating server.

  import roshttp.{HttpRequest, Protocol}

  val baseRequest: HttpRequest = {
    val hostPort: Array[String] = document.location.host.split(":")
    val host: String = hostPort(0)
    val request1 = HttpRequest().withHost(host)
    val request2 =
      if (hostPort.length > 1) {
        request1.withPort(hostPort(1).toInt)
      } else {
        request1
      }
    if (document.location.protocol == "https:") {
      request2.withProtocol(Protocol.HTTPS)
    } else {
      request2
    }
  }
@hmil
Copy link
Owner

hmil commented Jun 24, 2018

Hi, thanks for the feedback.

How about using the constructor function with a relative url?

HttpRequest("/this-relative-endpoint").send()

@lj-ditrapani
Copy link
Author

lj-ditrapani commented Jun 25, 2018

Thanks for the quick response.

Yeah, I originally tried that and just tried again to make sure in both firefox and chrome. The host ends up being null and the port is not set. So HttpRequest("/status").send() causes the browser to request http://null/status.

The behavior seems consistent with the source.
https://github.com/hmil/RosHTTP/blob/master/shared/src/main/scala/fr/hmil/roshttp/HttpRequest.scala#L363-L366
The path & host are set to null and the protocol to HTTP.

https://github.com/hmil/RosHTTP/blob/master/shared/src/main/scala/fr/hmil/roshttp/HttpRequest.scala#L219-L224
Then withURL creates a URI from the relative path. The URI will return null for the scheme/host/port, so those remain set to null.

I'm using "2.1.0" with "sbt-scalajs" % "0.6.23".

@hmil
Copy link
Owner

hmil commented Jun 25, 2018

Hmm.. That's not right.

We should not allow requests to be constructed with a null host, because then the request's url is garbage.

The "withURL" method needs to change.
It should accept urls of the form (protocol:)?//host(:port)?(/path)?(?query)? (absolute, note that protocol can be omitted) and /path(?query)? (relative), but reject others with an exception. (Yes, this is not a comprehensive handling of urls, but we'll let more exotic forms aside).

Additionally, the drivers API should be extended with a method used to provide default values when these can not be found in the URL. Something like:

getDefaultProtocol(): String
getDefaultHost(): String
getDefaultPort(): Integer

In the browser, we can use window.location to figure this out. In server-side environments, defaultProtocol should be HTTPS, while defaultHost and defaultPort should both throw an exception.

PRs welcome, I am not contributing code here anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants