spring-cloud-netflix icon indicating copy to clipboard operation
spring-cloud-netflix copied to clipboard

Published 20 hours ago • verified publisher icon spring-cloud

Update ZuulHandlerMapping to use PathMatcher

Open salavessa opened this issue 8 years ago • 4 comments

Zuul ignored patterns have inconsistent behaviour on edge cases

fixes gh-1575

salavessa avatar Dec 24 '16 00:12 salavessa

Current coverage is 74.13% (diff: 0.00%)

Merging #1576 into master will decrease coverage by 0.09%

@@             master      #1576   diff @@
==========================================
  Files           193        193          
  Lines          5937       5939     +2   
  Methods           0          0          
  Messages          0          0          
  Branches        893        894     +1   
==========================================
- Hits           4407       4403     -4   
- Misses         1206       1212     +6   
  Partials        324        324

Powered by Codecov. Last update 1cd194a...e1e61b5

codecov-io avatar Dec 24 '16 00:12 codecov-io

Sorry, can't see any logical test fitting in for this fix. The problem is that values from the same object (ZuulProperties.ignoredPatterns) and whithin the same flow are being checked against two different implementations (probably inadvertently) which are clearly not 100% compatible (PatternMatchUtils.simpleMatch vs AntPathMatcher.match) as one is pure wildcard and the other has embedded path logic. If you are looking for some cases where the used implementations fail to match, please check below:

PATTERN '/*'
	'/p1/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1*'
	'/p1/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*'
	'/p1/p2/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/**'
	'/p1' AntPathMatcher=true; MatchUtils=false
PATTERN '/p*'
	'/p1/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p3*'
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p3**'
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p3/*'
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p3/**'
	'/p1/p2/p3' AntPathMatcher=true; MatchUtils=false
PATTERN '/p1/**/p3'
	'/p1/p2/p3/' AntPathMatcher=true; MatchUtils=false
PATTERN '/p1/**/p3*'
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/**/p3**'
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/**/p3/'
	'/p1/p2/p3' AntPathMatcher=true; MatchUtils=false
PATTERN '/p1/**/p3/*'
	'/p1/p2/p3/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/**/p3/**'
	'/p1/p2/p3' AntPathMatcher=true; MatchUtils=false
PATTERN '/p1/*/p4'
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p4*'
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p4**'
	'/p1/p2/p3/p4' AntPathMatcher=false; MatchUtils=true
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p4/'
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p4/*'
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/*/p4/**'
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/**/p4'
	'/p1/p2/p3/p4/' AntPathMatcher=true; MatchUtils=false
PATTERN '/p1/**/p4/'
	'/p1/p2/p3/p4' AntPathMatcher=true; MatchUtils=false
PATTERN '/p1/**/p4/*'
	'/p1/p2/p3/p4/' AntPathMatcher=false; MatchUtils=true
PATTERN '/p1/**/p4/**'
	'/p1/p2/p3/p4' AntPathMatcher=true; MatchUtils=false

Thanks

salavessa avatar Dec 27 '16 17:12 salavessa

rebuilding

spencergibb avatar Jan 09 '17 18:01 spencergibb

I'm hesitant to make a change like this without a test. If you can't come up with one, I'll take a look later.

spencergibb avatar Jan 09 '17 18:01 spencergibb

This module has entered maintenance mode. This means that the Spring Cloud team will no longer be adding new features to the module.

spencergibb avatar Jan 12 '23 15:01 spencergibb