Add Support & Tests for X-Forwarded-Path

[wooliet]

zetta.js : updated to include useXForwardedRootHeader option when
creating HttpServer instance.

lib/http_server.js : updated to include useXForwardedRootHeader
option as part of call to titan

test/test_api.js : updated with new tests that include the new
"X-Forwarded-Root" header as well as more tests for the existing
"X-Forwarded-Host" header. Main additions are to parse href values
in the response body to check expected host or root path value.

---

PR depends on argo/titan#5 and argo/argo-url-helper#4

[AdamMagaluk]

Thanks for the PR and starting this discussion.

Could this been done at an api management layer in front of zetta by rewriting the response to match any new paths?

My main concern would be that we are introducing a non standard header/feature. X-Forwarded-Host has been around awhile (coming i think from the mod_proxy in Apache) but there hasn't been a need so far for forwarded path root.

However i think with how we use hypermedia linking in Siren as well through our websocket Siren responses there could be a need for something like this in the general api community. For example here is a post on the api-craft community list with a similar issue. https://groups.google.com/forum/#!topic/api-craft/PHjQzExL_5A

@kevinswiber @mdobson Any take on this?

[mdobson]

No additional thoughts. I think you sum up my thoughts on it as well.

-Matt

--
Matthew Dobson | apigee https://apigee.com/ | m: +1.734.634.5472 |
twitter @mdobs http://twitter.com/mdobs @apigee
https://twitter.com/apigee | Apigee Community
http://community.apigee.com/ for answers, ideas and support!

On Mon, Aug 15, 2016 at 12:09 PM, Adam Magaluk [email protected]
wrote:

Thanks for the PR and starting this discussion.

Could this been done at an api management layer in front of zetta by
rewriting the response to match any new paths?

My main concern would be that we are introducing a non standard
header/feature. X-Forwarded-Host has been around awhile (coming i think
from the mod_proxy in Apache) but there hasn't been a need so far for
forwarded path root.

However i think with how we use hypermedia linking in Siren as well
through our websocket Siren responses there could be a need for something
like this in the general api community. For example here is a post on the
api-craft community list with a similar issue. https://groups.google.com/
forum/#!topic/api-craft/PHjQzExL_5A

@kevinswiber https://github.com/kevinswiber @mdobson
https://github.com/mdobson Any take on this?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#320 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA5BsGzQuvM3PhO7vBj3hb9nDshYb284ks5qgI80gaJpZM4JdM3t
.

[kevinswiber]

The closest thing is X-Forwarded-Path. It exists in the wild but isn't as widely adopted as other X-Forwarded-* headers. Zetta uses these headers to do the URL rewriting internally, without the need of an API management layer interfering. I do see this as being a valid Zetta concern. At least it's one we've already adopted. This is just another facet.

I vote for using X-Forwarded-Path instead of X-Forwarded-Root, and we'd need to update this dependency accordingly, likely with a feature toggle: https://github.com/argo/argo-url-helper

[AdamMagaluk]

+1 for X-Forwarded-Path over X-Forwarded-Root

[wooliet]

I considered "path" but didn't want to imply it would also change the /server/{uuid} and other possible portions of the response. But of course I'd be happy to update my argo/argo-url-helper#4 PR and others to change it to whatever you guys think best.

[kevinswiber]

@wooliet I'll let @AdamMagaluk and @mdobson make the final decision here. If Zetta has a benevolent dictator, it's probably not me. Looking at the commit log, I'm mostly just an advisor these days, kept around mostly for historical value. ;)

[mdobson]

I'd prefer X-Forwarded-Path. Root has too many connotations for my liking.
Let's move forward with that. Once thing to possibly note is we'll also
need to compensate for this path in a few other places as well. Once your
PRs for titan and argo-url-helper have been resolved we'll walk you through
the next set of changes that will be needed in zetta.

Anything else @AdamMagaluk https://github.com/AdamMagaluk?

-Matt

--
Matthew Dobson | apigee https://apigee.com/ | m: +1.734.634.5472 |
twitter @mdobs http://twitter.com/mdobs @apigee
https://twitter.com/apigee | Apigee Community
http://community.apigee.com/ for answers, ideas and support!

On Mon, Aug 15, 2016 at 4:38 PM, Kevin Swiber [email protected]
wrote:

@wooliet https://github.com/wooliet I'll let @AdamMagaluk
https://github.com/AdamMagaluk and @mdobson https://github.com/mdobson
make the final decision here. If Zetta has a benevolent dictator, it's
probably not me. Looking at the commit log, I'm mostly just an advisor
these days, kept around mostly for historical value. ;)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#320 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA5BsFnb0WybhPY_uHpAwuP864iTiB3gks5qgM41gaJpZM4JdM3t
.

[AdamMagaluk]

+1 for Matt's decision. Let's move forward with path in the argo packages then figure out where we need to change zetta.

[mdobson]

Just for the sake of listing:

1. We need to account for this header setting in rewriting links that
   are streamed over websockets
2. We need to have zetta account for this setting when accepting
   requests to the server.

Anything else I could be missing?

-Matt

--
Matthew Dobson | apigee https://apigee.com/ | m: +1.734.634.5472 |
twitter @mdobs http://twitter.com/mdobs @apigee
https://twitter.com/apigee | Apigee Community
http://community.apigee.com/ for answers, ideas and support!

On Mon, Aug 15, 2016 at 4:43 PM, Adam Magaluk [email protected]
wrote:

+1 for Matt's decision. Let's move forward with path in the argo packages
then figure out where we need to change zetta.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#320 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA5BsER34l2GL64xwT_IEh2KcQQrOXvFks5qgM9igaJpZM4JdM3t
.