Hi Claude,
As I recall, the intention was that if PatternType is ANY, the name field would
simply be ignored. Probably we should have thrown an exception if someone tried
to create a ResourcePatternFilter with a non-null name and PatternType ANY.
best,
Colin
On Tue, Aug 20, 2024, at 04:19, Claude Warren, Jr wrote:
> Should a ResourcePatternFilter that has a PatternType of ANY and a name of
> WILDCARD_RESOURCE not match any Acls? I think this is a bug.I am writing
> a series of tests to ensure that I have implemented everything correctly in
> the Trie implementation and this has come up.
>
> public boolean matches(ResourcePattern pattern) {
> if (!resourceType.equals(ResourceType.ANY) &&
> !resourceType.equals(pattern.resourceType())) {
> return false;
> }
>
> if (!patternType.equals(PatternType.ANY) &&
> !patternType.equals(PatternType.MATCH) &&
> !patternType.equals(pattern.patternType())) {
> return false;
> }
>
> if (name == null) {
> return true;
> }
>
> if (patternType.equals(PatternType.ANY) ||
> patternType.equals(pattern.patternType())) {
> return name.equals(pattern.name());
> }
>
> switch (pattern.patternType()) {
> case LITERAL:
> return name.equals(pattern.name()) ||
> pattern.name().equals(WILDCARD_RESOURCE);
>
> case PREFIXED:
> return name.startsWith(pattern.name());
>
> default:
> throw new IllegalArgumentException("Unsupported
> PatternType: " + pattern.patternType());
> }
> }
>
> The above code is from the ResourcePatternFilter.
> The first if checks for resource type not matching.
> the second if checks for pattern type not matching.
> the third if will return true if the name is null.
> the fourth if will return true if the name matches in the case where
> pattern type is ANY or has equality. The only possible conditions here are
> ANY, MATCH or equality.
> the switch handles the case where the pattern type does not match.I think
> this can be simplified as:
>
> public boolean matches(ResourcePattern pattern) {
> if (!resourceType.equals(ResourceType.ANY) &&
> !resourceType.equals(pattern.resourceType())) {
> return false;
> }
>
> switch (pattern.patternType()) {
> case UNKNOWN:
> return patternType.equals(pattern.patternType());
>
> case ANY:
> case MATCH:
> return name == null || name.equals(pattern.name()) ||
> pattern.name().equals(WILDCARD_RESOURCE) ||
> name.equals(WILDCARD_RESOURCE);
>
> case LITERAL:
> return patternType.equals(pattern.patternType()) ||
> name == null || name.equals(pattern.name()) ||
> pattern.name().equals(WILDCARD_RESOURCE);
>
> case PREFIXED:
> return patternType.equals(pattern.patternType()) ||
> name == null || name.startsWith(pattern.name()) ||
> pattern.name().equals(WILDCARD_RESOURCE);
>
> default:
> throw new IllegalArgumentException("Unsupported
> PatternType: " + pattern.patternType());
> }
> }
>
>
> The above case still handles the null name and handles the
> WILDCARD_RESOURCE as a valid pattern name. I also think it is simpler to
> read.
>
> Am I missing something?
