Skip to content

Conditionally add headers.#3047

Merged
ryanjbaxter merged 2 commits intospring-cloud:masterfrom
dev-priporov:issue-3036
Jul 2, 2018
Merged

Conditionally add headers.#3047
ryanjbaxter merged 2 commits intospring-cloud:masterfrom
dev-priporov:issue-3036

Conversation

@dev-priporov
Copy link
Copy Markdown
Contributor

@dev-priporov dev-priporov commented Jun 30, 2018

ignored headers are added in method buildZuulRequestHeaders in class ProxyRequestHelper in code below:

for (String header : zuulRequestHeaders.keySet()) {
	headers.set(header, zuulRequestHeaders.get(header));
}

I've added a condition to skip these headers:

for (String header : zuulRequestHeaders.keySet()) {
	if (isIncludedHeader(header)){
		headers.set(header, zuulRequestHeaders.get(header));
	}
}

Fixes gh-3036

@pivotal-issuemaster
Copy link
Copy Markdown

@dev-priporov Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spencergibb spencergibb changed the title Fix issues-3036 Conditionally add headers. Jun 30, 2018
@pivotal-issuemaster
Copy link
Copy Markdown

@dev-priporov Thank you for signing the Contributor License Agreement!

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 30, 2018

Codecov Report

Merging #3047 into master will increase coverage by 0.43%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3047      +/-   ##
============================================
+ Coverage     65.88%   66.32%   +0.43%     
- Complexity     1439     1440       +1     
============================================
  Files           182      182              
  Lines          6750     6755       +5     
  Branches        813      814       +1     
============================================
+ Hits           4447     4480      +33     
+ Misses         1994     1966      -28     
  Partials        309      309
Impacted Files Coverage Δ Complexity Δ
...cloud/netflix/zuul/filters/ProxyRequestHelper.java 90.69% <50%> (-0.71%) 54 <0> (ø)
...amework/cloud/netflix/zuul/web/ZuulController.java 87.5% <0%> (-12.5%) 2% <0%> (ø)
...netflix/ribbon/ZonePreferenceServerListFilter.java 61.53% <0%> (-11.54%) 9% <0%> (-1%)
...oud/netflix/ribbon/okhttp/OkHttpRibbonRequest.java 78.72% <0%> (-1.72%) 8% <0%> (ø)
...strix/dashboard/HystrixDashboardConfiguration.java 25.24% <0%> (-0.25%) 5% <0%> (ø)
.../netflix/zuul/filters/post/SendResponseFilter.java 80% <0%> (+0.14%) 35% <0%> (ø) ⬇️
...etflix/turbine/stream/HystrixStreamAggregator.java 78.94% <0%> (+5.26%) 9% <0%> (+1%) ⬆️
...cloud/netflix/eureka/EurekaInstanceConfigBean.java 69.03% <0%> (+16.75%) 54% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13c2500...4b22686. Read the comment docs.

@dev-priporov
Copy link
Copy Markdown
Contributor Author

are all success checks necessary to complete PR?

@ztgoto
Copy link
Copy Markdown

ztgoto commented Dec 2, 2020

client -> zuul proxy -> api service.

After this change, how can zuul proxy not allow forwarding client header information to the api service but allow zuul to generate the same header information to the api service, because the client is not trusted, but the zuul proxy is trusted

@ryanjbaxter
Copy link
Copy Markdown
Contributor

Please open a separate issue with a sample that demonstrates the problem

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.

PreDecorationFilter#addProxyHeaders append X-Forwarded-xx headers even if these headers are ignored

6 participants