Re: Suggestions

Hi Chris,

----- Original Message -----
From: "Chris Nokleberg" <chris-k7PiZI/[email protected]>
To: <securityfilter-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Sent: Thursday, December 19, 2002 12:08 PM
Subject: [securityfilter-user] Suggestions


> Hello,
>
> I'm looking to use securityfilter in a project soon. It seems like just
> what I need, but looking at the code I do have a few questions/comments:
>
> a) "replace '/' chars with '/+' to match one or more consecutive slashes"
>   seems wrong to me. Can you give me a pointer to the relevant portion
>   of the Servlet spec?

I made this change to account for the way the servers typically work. If you
write a
web page that normally has the URL '/admin/editEmployee.jsp' and someone
makes a request for '///////admin////editEmployee.jsp' (insert as many
slashes
as you want) they will see the page. SecurityFilter needs to protect that
page if you have a mapping like '/admin/*', so I added the + after the
slashes to match one or more of them.

>
> b) I really do not see a need for a regular expression library. Here is
> an alternative implementation:
>
>    private String matchAgainst; // new field
>    protected void initPatternType() {
>       if (pattern.startsWith("*.")) {
>          patternType = URLPattern.EXTENSION_TYPE;
>          matchAgainst = pattern.substring(1);
>       } else if (pattern.startsWith("/") && pattern.endsWith("/*")) {
>          patternType = URLPattern.PATH_TYPE;
>          matchAgainst = pattern.substring(0, pattern.length() - 1);
>       } else {
>          patternType = URLPattern.EXACT_TYPE;
>          matchAgainst = pattern;
>       }
>    }
>    // new method
>    protected boolean match(String path) {
>         switch (type) {
>         case PATH_TYPE:
>             return path.startsWith(matchAgainst);
>         case EXTENSION_TYPE:
>             return path.endsWith(matchAgainst);
>         default:
>             return path.equals(matchAgainst);
>         }
>     }
>
> I'm sure this is faster, and doesn't have threading issues (I have
> similar code in a web.xml parser I wrote recently).

That fails for a few of the tests (see the /regex-test CVS module in the
securityfilter repository). It fails for the one with multiple slashes (the
/+ thing from the first point above). For instance /////catalog should match
/catalog, but it doesn't. It also fails where /baz is supposed to match
/baz/*. I added some NoRegex* classes to the regex-test CVS module to test
the code above if you want to check it out.

I ended up using the Jakarta-ORO Perl5 patterns after testing the
performance and thread safety of the various options available. With the
tests that NoRegex fails removed from the test, the difference in matching
performance is definitely present  (25000 multi-threaded tests on P3 1GHz):
noregex: 6.7 seconds
oro.perl5: 9.3 seconds

There is a fair bit of overhead in the tests, so the matching itself is
probably much faster with NoRegex (say 2 seconds versus 5 is my guess), but
the overall difference in performance is close enough that I don't feel
compelled to get rid of Perl5. I am sure we could change the code to account
for the test cases that it failed, but I don't think it would end up being
much faster than Perl5. It might even be slower. Using a regex package keeps
the code simpler, too.

If you want to code a compliant matching routine to see how it performs, you
can test it with the regex-test framework, or email it to me and I'll test
it. It would be nice to reduce the number of external libraries if we can
get a correct and performant solution without a regex package.

>
> c) URLPattern is missing the concept of the "default" servlet. A
> URLPattern of "/" should match against any path (after all exact, path,
> and extension mappings have failed, of course). See section 11.2 of the
> Servlet 2.3 spec.

Good catch. I need to add a default mapping type for / and match that last.

On a related note, I need to add another type for form-login-page and
form-error-page and match those first with no authentication requirements. I
might add a type for j_security_check, too and refactor the code to just try
matching any of these and do the processing based on what gets matched.

>
> d) What is the status of user-data-constraint support?

There hasn't been much interest expressed so far, so I haven't even thought
about it
much. I would really like to get it into the project for compliance with the
spec. Code contributions are welcome!

-Max

>
> I'm happy to produce patches for any or all of these things if you
> agree.
>
> Thanks,
> Chris
>
>
> -------------------------------------------------------
> This SF.NET email is sponsored by: Geek Gift Procrastinating?
> Get the perfect geek gift now!  Before the Holidays pass you by.
> T H I N K G E E K . C O M      http://www.thinkgeek.com/sf/
> _______________________________________________
> securityfilter-user mailing list
> securityfilter-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/securityfilter-user
>





-------------------------------------------------------
This SF.NET email is sponsored by:  The Best Geek Holiday Gifts!
Time is running out!  Thinkgeek.com has the coolest gifts for
your favorite geek.   Let your fingers do the typing.   Visit Now.
T H I N K G E E K . C O M        http://www.thinkgeek.com/sf/