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

Overriding port for absolute refs when run from browser [3.1.2] #29

Closed
andru opened this issue Nov 22, 2016 · 4 comments
Closed

Overriding port for absolute refs when run from browser [3.1.2] #29

andru opened this issue Nov 22, 2016 · 4 comments

Comments

@andru
Copy link

andru commented Nov 22, 2016

I've got a schema with absolute urls to resources with no port number in the URL...

{
  "$schema": "http:https://json-schema.org/draft-04/schema#",
  "id": "http:https://localhost/schema/01/plantdb.json",
  "type": "object",
  "properties": {
    "names": {"$ref": "http:https://localhost/schema/01/names.json"}
  }
}

(greatly simplified for clarity)

and I'm loading this on the client from an app hosted at localhost:3000. Trying to dereference a schema file hosted on the same hostname, but without specifying a port number...

parser.dereference('http:https://localhost/schema/01/plantdb.json')

actually tries to load the schema from http:https://localhost:3000/schema/01/plantdb.json and fails with an invalid schema error. If I explicitly add the port number to the schema URL:

parser.dereference('http:https://localhost:80/schema/01/plantdb.json')

then that schema file loads, but all refs within are still loaded from port http:https://localhost:3000.

Is this a bug, or is there a config somewhere I'm missing to force the port number on ref requests?

@JamesMessinger
Copy link
Member

JamesMessinger commented Nov 24, 2016

There's nothing in JSON Schema $Ref Parser that would add port 3000 to URLs. You can verify this yourself by checking the URL at this point in the code to see the HTTP request that is being sent.

Perhaps the server you're using is doing some redirect or proxying behavior?

@andru
Copy link
Author

andru commented Nov 25, 2016

I've done some debugging and I think there's a bug here.

This script is running on a page served from localhost:3000 wherein I want to dereference a schema from a http URL...

parser.dereference('http:https://localhost/schema/01/plantdb.json')

quick sidenote: A simple fix here is appending :80 to the hostname, but then the bug just moves to the request for each ref URL within (which don't have port numbers, assuming the sensible default of 80 for http) which I ordinarily wouldn't have control over, so I'm keeping it simple here by leaving the port off the initial request

This URL gets parsed to the following object as a function parameter to get in lib/resolvers/http.js

{
  "protocol":"http:",
  "slashes":true,
   "auth":null,
  "host":"localhost",
  "port":null,
  "hostname":"localhost",
  "hash":null,
  "search":null,
  "query":null,
  "pathname":"/schema/01/plantdb.json",
  "path":"/schema/01/plantdb.json",
  "href":"http:https://localhost/schema/01/plantdb.json"
}

Note that port is null, and both host and hostname exist.

This is then used to create a new options object for http.get or https.get in lib/resolvers/http.js:142 which in my case comes out looking like this

    var req = protocol.get({
      hostname: localhost,
      port: null,
      path: "/schema/01/plantdb.json",
      auth: null,
      headers: {},
      withCredentials: false
    });

In the browser, the http and https node modules are polyfilled by http-browserify.

This is where the bug appears.

http-browserify looks for a host parameter. If it doesn't receive it, it copies over from hostname, but not until after it's already decided the absence of both host and port means it should copy the port from the current page URL http.request method...

    if (!params.host && !params.port) {
        params.port = parseInt(window.location.port, 10);
    }
    if (!params.host && params.hostname) {
        params.host = params.hostname;
    }

I'm not sure whether this should be considered a bug in http-browserify or this project.

According to the Node docs for http.request:

host [String] A domain name or IP address of the server to issue the request to. Defaults to 'localhost'.
hostname [String] Alias for host. To support url.parse(), hostname is preferred over host.

According to http-browserify.request docs, the parameter should be host. hostname is not mentioned.

opts.host=window.location.host - http host

Whatever the case, if browserify-http.request doesn't see a host or a port, it will default to the port of the current page. It seems like an easy fix in this repo would be to pass both hostname and host, or to default port to 80 if no other value is set in the URL.

I could submit a pull request if you agree.

@JamesMessinger
Copy link
Member

@andru - Wow, thanks for the thorough explanation! I like your idea of setting both hostname and host. That's probably the best solution for now, and I'd gladly accept a PR from you.

Whenever I get a chance, I plan to resume working on JSON Schema $Ref Parser v4.0, and one of the changes that I plan to make is getting rid of HTTP-Browserify, probably in favor of Axios or something

@philsturgeon
Copy link
Member

PR was requested in 2016 but never received, so have to assume this either works now or demand is not large.

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

No branches or pull requests

3 participants