struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Pratt <thechrispr...@gmail.com>
Subject Re: Bug in CookieInterceptor
Date Fri, 19 Dec 2014 16:28:58 GMT
Sorry, forgot to log the bug.  Here it is
https://issues.apache.org/jira/browse/WW-4437
  (*Chris*)
On Tue Dec 16 2014 at 10:42:42 PM Lukasz Lenart <lukaszlenart@apache.org>
wrote:

> Hi Chris,
>
> Thanks for pointing this out and feel free to register an issue.
> Indeed all your comments are valid but instead of implementing them I
> would rather switch to AcceptedPatternsChecker (the same as for
> ParametersInterceptor) - frankly, I was sure I already did that :(
>
> 2014-12-16 22:43 GMT+01:00 Chris Pratt <thechrispratt@gmail.com>:
> > Sorry, I'm sure this is getting annoying by now, but  It looks like the
> > default and the override are handled differently.  The current code
> > compiles the default case insensitive, but not the passed in pattern.
> > Should that be consistent?
> >
> > private Pattern acceptedPattern =
> > Pattern.compile(ACCEPTED_PATTERN,Pattern.CASE_INSENSITIVE);
> >
> > public void setAcceptCookieNames (String pattern) {
> >   acceptedPattern = Pattern.compile(pattern);
> > }
> >
> >
> > On Tue Dec 16 2014 at 1:38:45 PM Chris Pratt <thechrispratt@gmail.com>
> > wrote:
> >
> >> Oh, on another note, it would be ever so slightly more efficient to
> invert
> >> the tests.  You are always testing both matches && LOG.isTraceEnabled()
> in
> >> the current implementation, but there's no need to check matches if
> trace
> >> isn't enabled.  So, maybe something like:
> >>
> >> /**
> >>  * Checks if name of Cookie match {@link #acceptedPattern}
> >>  *
> >>  * @param name of Cookie
> >>  * @return true|false
> >>  */
> >> protected boolean isAccepted(String name){
> >>   boolean matches = acceptedPattern.matcher(name).matches();
> >>
> >>   if(LOG.isTraceEnabled()) {
> >>     if(matches) {
> >>
> >>
> >>       LOG.trace("Cookie [#0] matches acceptedPattern
> [#1]",name,acceptedPattern.pattern());
> >>     } else {
> >>
> >>       LOG.trace("Cookie [#0] doesn't match acceptedPattern
> [#1]",name,acceptedPattern.pattern());
> >>     }
> >>   }
> >>   return matches;
> >> }
> >>
> >>
> >> On Tue Dec 16 2014 at 1:28:38 PM Chris Pratt <thechrispratt@gmail.com>
> >> wrote:
> >>
> >>> Sorry, I don't have an environment set up to create a patch, but I
> found
> >>> an error in the isAccepted() method.  It currently looks like:
> >>>
> >>> /**
> >>>  * Checks if name of Cookie match {@link #acceptedPattern}
> >>>  *
> >>>  * @param name of Cookie
> >>>  * @return true|false
> >>>  */
> >>> protected boolean isAccepted(String name) {
> >>>     boolean matches = acceptedPattern.matcher(name).matches();
> >>>     if (matches) {
> >>>         if (LOG.isTraceEnabled()) {
> >>>             LOG.trace("Cookie [#0] matches acceptedPattern [#1]",
> name, ACCEPTED_PATTERN);
> >>>         }
> >>>     } else {
> >>>         if (LOG.isTraceEnabled()) {
> >>>             LOG.trace("Cookie [#0] doesn't match acceptedPattern
> [#1]", name, ACCEPTED_PATTERN);
> >>>         }
> >>>     }
> >>>     return matches;
> >>> }
> >>>
> >>>
> >>> But it would be more useful if it actually reported the RegEx being
> used
> >>> instead of the default.  So something more like:
> >>>
> >>> /**
> >>>  * Checks if name of Cookie match {@link #acceptedPattern}
> >>>  *
> >>>  * @param name of Cookie
> >>>  * @return true|false
> >>>  */
> >>> protected boolean isAccepted (String name) {
> >>>   boolean matches = acceptedPattern.matcher(name).matches();
> >>>   if(matches) {
> >>>     if(LOG.isTraceEnabled()) {
> >>>       LOG.trace("Cookie [#0] matches acceptedPattern
> [#1]",name,acceptedPattern.pattern());
> >>>     }
> >>>   } else {
> >>>     if(LOG.isTraceEnabled()) {
> >>>       LOG.trace("Cookie [#0] doesn't match acceptedPattern
> [#1]",name,acceptedPattern.pattern());
> >>>     }
> >>>   }
> >>>   return matches;
> >>> }
> >>>
> >>>   (*Chris*)
> >>>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message