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 <thechrispr...@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 <thechrispr...@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 <thechrispr...@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-unsubscr...@struts.apache.org
For additional commands, e-mail: dev-h...@struts.apache.org
