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

Include the "scheme" in self links #77

Merged
merged 2 commits into from Dec 15, 2017

Conversation

Projects
None yet
3 participants
@bsnow-shl
Copy link
Contributor

commented Dec 7, 2017

Rdap self links do not include the "scheme" - the protocol prefix like http:// or https:// - so they are neither valid relative nor absolute links (referred to as suffix references in rfc 3986). This PR adds the "scheme" and the "://" to each of the Servlets, as part of the variable called "header". The header goes nowhere other than the Link object, so this seems to work well and maintain consistency.

This change is RFC compliant, and brings reddog in line with verisign rdap, as well as all the usual REST services. The following is an example of a new links section:

  "links": [
    {
      "value": "https://example.com/entity/22-handle",
      "rel": "self",
      "href": "https://example.com/entity/22-handle",
      "type": "application/rdap+json"
    }
  ]

See also:
links section example includes scheme: https://tools.ietf.org/html/rfc7483#section-4.2
suffix references are discouraged: https://tools.ietf.org/html/rfc3986#section-4.5
verisign rdap example: https://rdap-pilot.verisignlabs.com/rdap/v1/entity/299~VRSN

@pcarana pcarana self-assigned this Dec 8, 2017

@pcarana pcarana self-requested a review Dec 8, 2017

@pcarana

This comment has been minimized.

Copy link
Collaborator

commented Dec 8, 2017

Thanks for contribute with this fix. Just as stated, the change is RFC compliant, so we will work on it to release it as soon as possible.

@pcarana

This comment has been minimized.

Copy link
Collaborator

commented Dec 8, 2017

In order to complete the proposal, the commit 5f251bb has been made to consider that the server can be behind a proxy/balancer.

Now the scheme will be retrieved considering HTTP headers "Forwarded" (optional and defined at RFC 7239 section 5.4) and "X-Forwarded-Proto" (a de facto standard); headers that could be sent by a proxy/balancer. If none of those headers is sent, then the function getScheme() is used to get the scheme.

@bsnow-shl

This comment has been minimized.

Copy link
Contributor Author

commented Dec 8, 2017

This is a very thoughtful addition, and it continues to work with my code. I have been testing with registrobr/rdap-client

Thanks!

@pcarana

This comment has been minimized.

Copy link
Collaborator

commented Dec 8, 2017

Nice! Thanks for the help, now looking for the team's approval.

@pcarana pcarana requested a review from TheRedTrainer Dec 8, 2017

@pcarana

pcarana approved these changes Dec 8, 2017

Copy link
Collaborator

left a comment

Looks good, anything to add @dhfelix ? Also looking for review of @TheRedTrainer .

@pcarana pcarana added this to the v1.2.2 milestone Dec 12, 2017

@pcarana pcarana merged commit 5f251bb into NICMx:master Dec 15, 2017

@TheRedTrainer
Copy link

left a comment

Verified. The scheme is included in self links, even if the request is managed by a proxy server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.