2013/1/9 Remy Maucherat <r...@apache.org>:
> Hi,
>
> I ported a valve providing mod_rewrite functionality (most of it) for
> Tomcat "8", and committed it in the sandbox. This could be easily placed
> in the main repository, where it would provide an additional item in the
> "new features" department for this major release.
>
> I have a documentation page for it that could be included. The basics is
> that it uses a rewrite.properties that has the same contents as the
> configuration for mod_rewrite. The rewrite.properties is placed in the
> host config folder if the valve is declared in a Host, or in WEB-INF in
> the webapp if declared in a Context.
>
> The main differences with mod_rewrite are:
> - (the big one) no proxy flag
> - no SSL attributes
> - less file related flags available
> - allows virtual host rewriting using a host flag (it replaces the host
> header instead of the URL and maps again)
> Point 2 and 3 are probably relatively easy to fix, while obviously a
> proxy is a more involving endeavor.
>
> There are rewrite solutions out there already, but this one does not use
> complex tricks (because it can simply start over request processing
> instead of doing more complex request dispatching, which also impacts
> the Servlet state), and is mod_rewrite compatible.
>
> Comments ? Is it nice or useless ?
>

1. First concern that I see here is how is this compatible with
AccessLogValve? The valve should log original request, and what
happens if you rewrite it?

2. Is it compatible with async processing?


3. Generally, having such a feature would be useful, but I am afraid
that it will be abused.

I would like to deem it as "experimental" and to continue to promote
url-rewrite filter as the recommended solution.


Several comments on the code:

4.
> protected String resourcePath = "rewrite.properties"

I do not like the file name "rewrite.properties" as it is not a Java
properties file. Maybe "rewrite.config" ?

5.
> new InputStreamReader(is)

Using an InputStreamReader like the above to read configuration files
is broken. You have to specify an encoding explicitly, e.g. UTF-8.

I think "default encoding" is only useful when you are reading/writing
to a console. I other cases it is an oversight.

6.
> invoked.set(null);

Clearing the flag (the thread local variable) must be done in a finally{} block.

7.
> rules[i]
in every line...  Declare a local variable.


8. Maybe add a property to this valve named "enabled". The idea is
from AccessLogValve that has such a property. If its value is false,
pass the request through as is without any rewriting.




Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to