-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add null checks for org.springframework.boot.autoconfigure.web.ServerProperties.Servlet#getContextPath() #3262
base: main
Are you sure you want to change the base?
Conversation
…Properties.Servlet#getContextPath() Fixes spring-cloudgh-3259
Codecov Report
@@ Coverage Diff @@
## master #3262 +/- ##
============================================
+ Coverage 65.46% 65.95% +0.49%
- Complexity 1475 1478 +3
============================================
Files 188 188
Lines 6943 6949 +6
Branches 846 849 +3
============================================
+ Hits 4545 4583 +38
+ Misses 2085 2052 -33
- Partials 313 314 +1
Continue to review full report at Codecov.
|
How about a test? |
Ping @candrews, could you add a test? |
@@ -89,15 +89,19 @@ public HasFeatures zuulFeature() { | |||
@Bean | |||
@ConditionalOnMissingBean(DiscoveryClientRouteLocator.class) | |||
public DiscoveryClientRouteLocator discoveryRouteLocator() { | |||
return new DiscoveryClientRouteLocator(this.server.getServlet().getContextPath(), this.discovery, this.zuulProperties, | |||
String contextPath = this.server.getServlet().getContextPath(); | |||
if(contextPath == null) contextPath = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed and out of scope for this PR.
@@ -106,7 +106,9 @@ public CompositeRouteLocator primaryRouteLocator( | |||
@Bean | |||
@ConditionalOnMissingBean(SimpleRouteLocator.class) | |||
public SimpleRouteLocator simpleRouteLocator() { | |||
return new SimpleRouteLocator(this.server.getServlet().getContextPath(), | |||
String contextPath = this.server.getServlet().getContextPath(); | |||
if(contextPath == null) contextPath = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed and out of scope for this PR.
return new PreDecorationFilter(routeLocator, this.server.getServlet().getContextPath(), this.zuulProperties, | ||
String contextPath = this.server.getServlet().getContextPath(); | ||
if(contextPath == null) contextPath = ""; | ||
return new PreDecorationFilter(routeLocator, contextPath, this.zuulProperties, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be removed and a StringUtils.hasText()
added here https://github.com/spring-cloud/spring-cloud-netflix/blob/master/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java#L181
Ping @candrews care to continue working on this PR? |
Fixes gh-3259