qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Keith Wall (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (QPID-4334) [Java broker] move the Firewall functionality into the ACL plugin
Date Fri, 28 Sep 2012 09:21:08 GMT

    [ https://issues.apache.org/jira/browse/QPID-4334?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13465491#comment-13465491

Keith Wall commented on QPID-4334:

Hi Phil

Looks good, I just have a couple of comments, I think most of which are actually against the
original implementation.

64 - Guard debug logging

77 - public setOperation, setObjectType, setProperties could be private and members final
107 - comment re compareTo seems spurious
112 - the refactoring means we now always evaluate operation, objectType and properties rather
than potentially short circuiting earlier.  Also could properties ever end up being null?

75 - load should be closing the underlying reader

68 - Add the IP that failed to lookup to the text of the AccessControlFirewallException
100 - I think we should be logging at warn the case where rDNS lookups are failing/timing
out.  I worry that a sporadic DNS failure might lead to mysterious Java Broker defect reports.


public String call()
  boolean success = false;
   String hn =  remote.getCanonicalHostName();
   success = true;
   return hn;
     if (!success)
        log.warn("Failed to get canonical for " + ip + ", DNS timeout or error.");.

100 - confusing braces
177 - seemingly unnecessary use of reflection. InetAddress.getByAddress has been part of the
API since 1.4

code style (brace positions)

> [Java broker] move the Firewall functionality into the ACL plugin
> -----------------------------------------------------------------
>                 Key: QPID-4334
>                 URL: https://issues.apache.org/jira/browse/QPID-4334
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Java Broker
>            Reporter: Robbie Gemmell
>            Assignee: Keith Wall
>             Fix For: 0.19
>         Attachments: 0001-QPID-4334-removed-the-firewall-plugin-and-moved-its-.patch,
> Firewall rules are currently expressed separately from ACls, but this plugin is effectively
an access control plugin and it is felt that the functionality would thus be better expressed
using ACL rules. Such request has been made in the past, including a patch for the C++ broker
as in use by a user. 
> The firewall functionality will be moved into the ACL plugin, and the firewall plugin

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

View raw message