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

Impossible to set proxy_set_header for default location #467

Merged
merged 1 commit into from
Dec 5, 2014
Merged

Impossible to set proxy_set_header for default location #467

merged 1 commit into from
Dec 5, 2014

Conversation

invliD
Copy link
Contributor

@invliD invliD commented Sep 29, 2014

When setting proxy_set_header in a vhost, it will add those entries directly to the server although the documentation says it will be added to the default location. Since the headers are not passed on to the location, the location will be generated with the fault set, even if the given proxy_set_header does not include those.

This PR fixes both issues.

Travis fails because of unrelated issues.

@3flex
Copy link
Contributor

3flex commented Oct 1, 2014

Hi @invliD I can't see anywhere where it's documented that proxy_set_header will be added to the default location. Could you tell me where that's documented? Or if you're not using 0.1.0 or the master branch, tell me which version of the module you're using?

I'm inclined to update the documentation to reflect the true state of the code rather than change the behavior but I'd like to know where it says that.

@invliD
Copy link
Contributor Author

invliD commented Oct 1, 2014

I can't find it anymore, maybe I just made that up. Anyway, there are still inconsistencies since proxy_set_body and every other proxy_* directive to a vhost is actually added to the default location. proxy_set_header seems to be the only exception.

@mckern
Copy link

mckern commented Oct 22, 2014

I'm bumping up against this exact same problem right now. Any movement on this?

@invliD
Copy link
Contributor Author

invliD commented Dec 1, 2014

@3flex Any news?

@3flex
Copy link
Contributor

3flex commented Dec 4, 2014

@invliD can you please rebase, then delete the test at line 201 of spec/defines/resource_vhost_spec.rb? Once done I'll merge. Thanks!

@invliD
Copy link
Contributor Author

invliD commented Dec 5, 2014

@3flex Done.

3flex added a commit that referenced this pull request Dec 5, 2014
Impossible to set proxy_set_header for default location
@3flex 3flex merged commit ea94229 into voxpupuli:master Dec 5, 2014
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

Successfully merging this pull request may close these issues.

3 participants