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

request.originalUrl does not include the first path segment #57

Closed
jamestharpe opened this issue Oct 11, 2018 · 7 comments
Closed

request.originalUrl does not include the first path segment #57

jamestharpe opened this issue Oct 11, 2018 · 7 comments
Assignees

Comments

@jamestharpe
Copy link

request.originalUrl does not include the stage path segment from API Gateway.

Example of expected value: /api-gateway-stage/my/route

Example of actual value: /my/route

The correct path value is indicated in event.requestContext.path

@dougmoscrop
Copy link
Owner

Yeah, I think this is related to https://github.com/dougmoscrop/serverless-http/pull/36/files

The requestContex.path -- I thought it was "wrong" when you used a custom domain?

I could be remembering wrong, or it got fixed/changed, either way, I will get some sites deployed and figure out what works best for everyone (with and without stage names, custom domains, etc)

@dougmoscrop dougmoscrop self-assigned this Oct 11, 2018
@bsdkurt
Copy link
Contributor

bsdkurt commented Oct 11, 2018

I still think my pull request #42 is the correct fix for this. However, since it would require changes for existing users who don't use custom domains, it would need to be a major bump of serverless-http with an explanation of how to upgrade.

@dougmoscrop
Copy link
Owner

I think it's possible to build on it and make it do some kind of detection

@bsdkurt
Copy link
Contributor

bsdkurt commented Oct 24, 2018

The requestContex.path -- I thought it was "wrong" when you used a custom domain?

The problem with #42 was that people not using custom domains (Amazon endpoints) were not expecting the change in the path and their current code required changes to deal with the corrected behavior. Also offline support broke for them, most likely for the same reason but needs further investigation. See #42 (comment) for more details.

This #42 (comment) outlines the three use cases for endpoints and how the event.path and event.requestContext.path values change for each of the cases.

If you are interested in taking another look, I can update #42 for head.

@dougmoscrop
Copy link
Owner

I'm still trying to figure out if this is solvable in a way that makes everyone happy.

@dougmoscrop
Copy link
Owner

dougmoscrop commented Aug 13, 2019

There's new baseUrl support coming in the next version, which will normalize the path being sent to your application. I think the only way this can work is if you configure your application and this library to understand their mount point -- it's no different than trying to run an application behind nginx mounted underneath some additional /prefix -- it can work, but all the URLs will be messed up, and rewriting message bodies on the fly is not the business I want to be in!

Edit:

Just to reiterate what I've said elsewhere: originalUrl is an express concept, not a Node concept. In previous versions we were setting originalUrl and/or baseUrl on the req object (which is an instanceof http.IncomingMessage) this has never had any effect.

@dougmoscrop
Copy link
Owner

The correct way to solve this is to use the basePath option introduced in 2.1.0 and use some kind of environment variable to set it, and configure it in all your application logic that generates URLs.

I'm pretty sure it is impossible to have an application in API Gateway that seamlessly works for both custom domains and bare (execute-api) endpoints that have a different base path, i.e.

✔️

  • myapi.com/dev/foo
  • execute-api.amazonaws.com/dev/foo

  • myapi.com/foo
  • execute-api.amazonaws.com/dev/foo

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