2009/10/29 <ma...@apache.org>:
> Author: markt
> Date: Thu Oct 29 19:26:52 2009
> New Revision: 831069
>
> URL: http://svn.apache.org/viewvc?rev=831069&view=rev
> Log:
> Add an option to strictly enforce cookie naming rules.
> I'm not wild about the implementation but since we can't change the API, this
> was the best I could come up with. Suggestions for improvements welcome.
>
> Modified:
> tomcat/trunk/java/javax/servlet/http/Cookie.java
> tomcat/trunk/webapps/docs/config/systemprops.xml
> private static final String tspecials = ",; ";
> + private static final String tspecials2 = "()<>@,;:\\\"/[]?={} \t";
> + private static final String tspecials2NoSlash = "()<>@,;:\\\"/[]?={} \t";
1) What is the difference between tspecials2 and tspecials2NoSlash.
The above two values are the same ("()<>@,;:\\\"/[]?={} \t")
2) I think that you can do
private static final String tspecials2NoSlash = "()<>@,;:\\\"[]?={} \t";
private static final String tspecials2WithSlash = tspecials2NoSlash + "/";
(and compiler will calculate the second constant).
3)
and later
> + (STRICT_NAMING && !FWD_SLASH_IS_SEPARATOR &&
> + tspecials2NoSlash.indexOf(c) != -1) ||
> + (STRICT_NAMING && FWD_SLASH_IS_SEPARATOR &&
> + tspecials2.indexOf(c) != -1)) {
We can preselect the effective value for tspecials2:
e.g. (but better to move into the above static{} block):
private static final String tspecials2 = FWD_SLASH_IS_SEPARATOR ?
tspecials2WithSlash : tspecials2NoSlash;
Thus the condition will become more simple,
(STRICT_NAMING && tspecials2.indexOf(c) != -1)) {
>
> -
> + /**
> + * If set to true, we parse cookies strictly according to the servlet,
> + * cookie and HTTP specs by default.
> + */
> + private static final boolean STRICT_SERVLET_COMPLIANCE;
> +
> + /**
> + * If set to true, the <code>/</code> character will be treated as a
> + * separator. Default is usually false. If
> STRICT_SERVLET_COMPLIANCE==true
> + * then default is true. Explicitly setting always takes priority.
> + */
> + private static final boolean FWD_SLASH_IS_SEPARATOR;
> +
> + /**
> + * If set to false, we don't use the IE6/7 Max-Age/Expires work around.
> + * Default is usually true. If STRICT_SERVLET_COMPLIANCE==true then
> default
> + * is false. Explicitly setting always takes priority.
> + */
> + private static final boolean STRICT_NAMING;
The above JavaDoc looks just copied over from the ALWAYS_ADD_EXPIRES
property, and thus makes no sense here.
In the docs, and in your implementation below, the usual default is
false, and the compliance mode default is true, and the above JavaDoc
says the opposite.
> +
> +
> + static {
> + STRICT_SERVLET_COMPLIANCE = Boolean.valueOf(System.getProperty(
> + "org.apache.catalina.STRICT_SERVLET_COMPLIANCE",
> + "false")).booleanValue();
> +
> + String fwdSlashIsSeparator = System.getProperty(
> +
> "org.apache.tomcat.util.http.ServerCookie.FWD_SLASH_IS_SEPARATOR");
> + if (fwdSlashIsSeparator == null) {
> + FWD_SLASH_IS_SEPARATOR = STRICT_SERVLET_COMPLIANCE;
> + } else {
> + FWD_SLASH_IS_SEPARATOR =
> + Boolean.valueOf(fwdSlashIsSeparator).booleanValue();
> + }
> +
> + String strictNaming = System.getProperty(
> + "javax.servlet.http.Cookie.STRICT_NAMING");
> + if (strictNaming == null) {
> + STRICT_NAMING = STRICT_SERVLET_COMPLIANCE;
> + } else {
> + STRICT_NAMING =
> + Boolean.valueOf(strictNaming).booleanValue();
> + }
> +
> + }
> +
> +
>
(skipped)
Also,
"javax.servlet.http.Cookie. STRICT_NAMING"
It would be better to name that "org.apache.tomcat.util.http.
ServerCookie.STRICT_NAMING"
as I think java(x)* property names are reserved by Sun.
+ <code>org.apache.tomcat.util.http.ServerCookie.STRICT_NAMING</code>.
Note that the above line of the doc already uses the *.tomcat.*
property name, but everywhere else in the patch the javax.servlet.*
name was used.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
