← Back to team overview

linux-traipu team mailing list archive

[Bug 931917] Re: regex_policy should use apache like ALLOW/DENY

 

Hi Anshu

Thanks for the patch! Great to see you dive into Drizzle just like that
:-)

I reviewed your patch. But first I need to comment on the above
comments:

****
Ok, so we allow all of them. But we should still choose one as the recommended syntax (use this in examples and documentation) and the other should be considered backward compatiblity feature. So my proposal still is that ALLOW/DENY is the preferred pair actions and ACCEPT/REJECT is secondary.

With that in mind, I have a comment on the patch now:

*****

In policy.h:

> 104  return action == POLICY_ACCEPT ? "ACCEPT" : "DENY";

This should now return ALLOW/DENY (regardless of what the user has used
in the config file).

Alternatively, each PolicyItem should store the actual string that was
provided by the user and return that. (It seems this is only used to
print a debug message, so I'm for the above and simpler solution.)

Apart from this, it looks good. For your GSoC application this is
already a great showcase. But before this can be merged, you need to
also add tests to cover the new action words and documentation. See
tests/ and docs/ directories in the regex_policy directory.

-- 
You received this bug notification because you are a member of UBUNTU -
AL - BR, which is subscribed to Drizzle.
https://bugs.launchpad.net/bugs/931917

Title:
  regex_policy should use apache like ALLOW/DENY

Status in A Lightweight SQL Database for Cloud Infrastructure and Web Applications:
  New

Bug description:
  In auth_regex the pair of commands ACCEPT/DENY is confusing. It should
  have been either ACCEPT/REJECT  (see iptables) or ALLOW/DENY (see
  apache httpd). My proposal is to make it ALLOW/DENY.

  The fix should of course be backward compatible and still also support
  ACCEPT/DENY, but documentation should mark this usage as deprecated.

To manage notifications about this bug go to:
https://bugs.launchpad.net/drizzle/+bug/931917/+subscriptions


References