Skip to content
This repository has been archived by the owner on May 10, 2021. It is now read-only.

Copy host property from headers to multiValueHeaders #42

Closed
wants to merge 1 commit into from

Conversation

leomeneguzzi
Copy link
Contributor

Hi everyone, looking for a solution to to resolve #40 I did my fork, and case this is acceptable in the project, I will let my PR 😄

This PR takes the host property from event.headers and copy to host in event.multiValueHeaders.
I'm opening this PR just in case you think this a good way to solve this problem.

PS: If you think that has another way, just give the suggest and I will take a look and implement.
PS: I don't know if this change specifically needs some test, if need, what exactly I can test.

@FinnWoelm
Copy link
Collaborator

This is perfect, @leomeneguzzi! 🙌 We will merge this as soon as possible!

@s-kris
Copy link

s-kris commented Sep 30, 2020

@FinnWoelm It looks like the PR is missing a label and that caused the tests to fail. Can you look into this?

@lindsaylevine
Copy link
Contributor

@s-kris @leomeneguzzi i'll get this taken care of asap!

@abdelhamid-attaby
Copy link

@lindsaylevine could we merge this and publish it? It is very crucial to us.

@cassidoo cassidoo added the type: bug code to address defects in shipped code label Oct 1, 2020
@lindsaylevine lindsaylevine changed the base branch from master to main October 1, 2020 03:01
@cassidoo cassidoo self-requested a review October 1, 2020 03:02
Copy link
Contributor

@cassidoo cassidoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this and make a note to remove it when it is fixed internally!

@lindsaylevine
Copy link
Contributor

closing because of conflict in favor of #44 and getting out asap

@lindsaylevine
Copy link
Contributor

@s-kris @leomeneguzzi @abdelhamid-attaby this was just published!

@abdelhamid-attaby
Copy link

Thanks @lindsaylevine but I opened a same issue with x-forwarded-host header. Please check #46

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

req.headers.host is undefined on netlify deployment
6 participants