Author: rajith
Date: Wed May 12 01:48:38 2010
New Revision: 943351
URL: http://svn.apache.org/viewvc?rev=943351&view=rev
Log:
This commit contains a fix for QPID-2600
Added a test case to check if all allowed chars are accepted and the rest is
rejected.
Added a check for empty continuation lines.
Improved error reporting by adding a line number.
Removed test_allowed_chars_for_username method from acl.py as the check for
group name will flag the "@" char, making this test case redundent.
Modified:
qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp
qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.h
qpid/trunk/qpid/cpp/src/tests/acl.py
Modified: qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp?rev=943351&r1=943350&r2=943351&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.cpp Wed May 12 01:48:38 2010
@@ -297,13 +297,19 @@ int AclReader::read(const std::string& f
bool AclReader::processLine(char* line) {
bool ret = false;
std::vector<std::string> toks;
-
+
// Check for continuation
char* contCharPtr = std::strrchr(line, '\\');
bool cont = contCharPtr != 0;
if (cont) *contCharPtr = 0;
int numToks = tokenize(line, toks);
+
+ if (cont && numToks == 0){
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line \"" << lineNumber <<
"\" contains an illegal extension.";
+ return false;
+ }
+
if (numToks && (toks[0].compare("group") == 0 || contFlag)) {
ret = processGroupLine(toks, cont);
} else if (numToks && toks[0].compare("acl") == 0) {
@@ -317,7 +323,8 @@ bool AclReader::processLine(char* line)
if (ws) {
ret = true;
} else {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Non-continuation line
must start with \"group\" or \"acl\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " <<
lineNumber
+ << ", Non-continuation line must start with \"group\" or
\"acl\".";
ret = false;
}
}
@@ -341,32 +348,27 @@ int AclReader::tokenize(char* line, std
// If cont is true, then groupName must be set to the continuation group name
bool AclReader::processGroupLine(tokList& toks, const bool cont) {
const unsigned toksSize = toks.size();
+
if (contFlag) {
gmCitr citr = groups.find(groupName);
for (unsigned i = 0; i < toksSize; i++) {
- if (!checkName(toks[i])) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Name \"" <<
toks[i] << "\" contains illegal characters.";
- return false;
- }
if (!isValidUserName(toks[i])) return false;
addName(toks[i], citr->second);
}
} else {
if (toksSize < (cont ? 2 : 3)) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Insufficient tokens
for group definition.";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " <<
lineNumber
+ << ", Insufficient tokens for group definition.";
return false;
}
- if (!checkName(toks[1])) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Group name \"" <<
toks[1] << "\" contains illegal characters.";
+ if (!isValidGroupName(toks[1])) {
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Group name \"" << toks[1] << "\" contains
illegal characters.";
return false;
}
gmCitr citr = addGroup(toks[1]);
if (citr == groups.end()) return false;
for (unsigned i = 2; i < toksSize; i++) {
- if (!checkName(toks[i])) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Name \"" <<
toks[i] << "\" contains illegal characters.";
- return false;
- }
if (!isValidUserName(toks[i])) return false;
addName(toks[i], citr->second);
}
@@ -378,7 +380,8 @@ bool AclReader::processGroupLine(tokList
AclReader::gmCitr AclReader::addGroup(const std::string& newGroupName) {
gmCitr citr = groups.find(newGroupName);
if (citr != groups.end()) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Duplicate group name \""
<< newGroupName << "\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Duplicate group name \"" << newGroupName << "\".";
return groups.end();
}
groupPair p(newGroupName, nameSetPtr(new nameSet));
@@ -431,7 +434,8 @@ void AclReader::printNames() const {
bool AclReader::processAclLine(tokList& toks) {
const unsigned toksSize = toks.size();
if (toksSize < 4) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Insufficient tokens for
acl definition.";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Insufficient tokens for acl definition.";
return false;
}
@@ -439,7 +443,8 @@ bool AclReader::processAclLine(tokList&
try {
res = AclHelper::getAclResult(toks[1]);
} catch (...) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Unknown ACL permission
\"" << toks[1] << "\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Unknown ACL permission \"" << toks[1] << "\".";
return false;
}
@@ -449,7 +454,8 @@ bool AclReader::processAclLine(tokList&
if (actionAllFlag) {
if (userAllFlag && toksSize > 4) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Tokens found after
action \"all\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Tokens found after action \"all\".";
return false;
}
action = ACT_CONSUME; // dummy; compiler must initialize action for
this code path
@@ -457,7 +463,8 @@ bool AclReader::processAclLine(tokList&
try {
action = AclHelper::getAction(toks[3]);
} catch (...) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Unknown action \"" <<
toks[3] << "\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Unknown action \"" << toks[3] << "\".";
return false;
}
}
@@ -477,7 +484,8 @@ bool AclReader::processAclLine(tokList&
try {
rule->setObjectType(AclHelper::getObjectType(toks[4]));
} catch (...) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Unknown object
\"" << toks[4] << "\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " <<
lineNumber
+ << ", Unknown object \"" << toks[4] << "\".";
return false;
}
}
@@ -487,14 +495,17 @@ bool AclReader::processAclLine(tokList&
for (unsigned i=5; i<toksSize; i++) {
nvPair propNvp = splitNameValuePair(toks[i]);
if (propNvp.second.size() == 0) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Badly formed
property name-value pair \"" << propNvp.first << "\". (Must be name=value)";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " <<
lineNumber
+ <<", Badly formed property name-value pair \""
+ << propNvp.first << "\". (Must be name=value)";
return false;
}
Property prop;
try {
prop = AclHelper::getProperty(propNvp.first);
} catch (...) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Unknown property
\"" << propNvp.first << "\".";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " <<
lineNumber
+ << ", Unknown property \"" << propNvp.first <<
"\".";
return false;
}
rule->addProperty(prop, propNvp.second);
@@ -509,7 +520,8 @@ bool AclReader::processAclLine(tokList&
// If rule validates, add to rule list
if (!rule->validate(validationMap)) {
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Invalid
object/action/property combination.";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Invalid object/action/property combination.";
return false;
}
rules.push_back(rule);
@@ -528,10 +540,10 @@ void AclReader::printRules() const {
// Static function
// Return true if the name is well-formed (ie contains legal characters)
-bool AclReader::checkName(const std::string& name) {
+bool AclReader::isValidGroupName(const std::string& name) {
for (unsigned i=0; i<name.size(); i++) {
const char ch = name.at(i);
- if (!std::isalnum(ch) && ch != '-' && ch != '_' && ch != '@' && ch !=
'.') return false;
+ if (!std::isalnum(ch) && ch != '-' && ch != '_') return false;
}
return true;
}
@@ -548,11 +560,20 @@ AclReader::nvPair AclReader::splitNameVa
// Returns true if a username has the n...@realm format
bool AclReader::isValidUserName(const std::string& name){
- size_t pos = name.find('@');
+ size_t pos = name.find('@');
if ( pos == std::string::npos || pos == name.length() -1){
- errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Username '" <<
name << "' must contain a realm";
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " <<
lineNumber
+ << ", Username '" << name << "' must contain a realm";
return false;
}
+ for (unsigned i=0; i<name.size(); i++) {
+ const char ch = name.at(i);
+ if (!std::isalnum(ch) && ch != '-' && ch != '_' && ch != '@' && ch !=
'.' && ch != '/'){
+ errorStream << ACL_FORMAT_ERR_LOG_PREFIX << "Line : " << lineNumber
+ << ", Username \"" << name << "\" contains illegal
characters.";
+ return false;
+ }
+ }
return true;
}
Modified: qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.h
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.h?rev=943351&r1=943350&r2=943351&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/acl/AclReader.h Wed May 12 01:48:38 2010
@@ -109,7 +109,7 @@ class AclReader {
void printRules() const; // debug aid
bool isValidUserName(const std::string& name);
- static bool checkName(const std::string& name);
+ static bool isValidGroupName(const std::string& name);
static nvPair splitNameValuePair(const std::string& nvpString);
};
Modified: qpid/trunk/qpid/cpp/src/tests/acl.py
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/tests/acl.py?rev=943351&r1=943350&r2=943351&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/tests/acl.py (original)
+++ qpid/trunk/qpid/cpp/src/tests/acl.py Wed May 12 01:48:38 2010
@@ -117,36 +117,6 @@ class ACLTests(TestBase010):
except qpid.session.SessionException, e:
self.assertEqual(403,e.args[0].error_code)
-
- def test_group_and_user_with_same_name(self):
- """
- Test a group and user with same name
- Ex. group admin admin
- """
- aclf = ACLFile()
- aclf.write('group b...@qpid b...@qpid\n')
- aclf.write('acl deny b...@qpid bind exchange\n')
- aclf.write('acl allow all all')
- aclf.close()
-
- result = self.reload_acl()
- if (result.text.find("format error",0,len(result.text)) != -1):
- self.fail(result)
-
- session = self.get_session('bob','bob')
- try:
- session.queue_declare(queue="allow_queue")
- except qpid.session.SessionException, e:
- if (403 == e.args[0].error_code):
- self.fail("ACL should allow queue create request");
- self.fail("Error during queue create request");
-
- try:
- session.exchange_bind(exchange="amq.direct", queue="allow_queue",
binding_key="routing_key")
- self.fail("ACL should deny queue bind request");
- except qpid.session.SessionException, e:
- self.assertEqual(403,e.args[0].error_code)
-
#=====================================
# ACL file format tests
@@ -185,23 +155,26 @@ class ACLTests(TestBase010):
"""
aclf = ACLFile()
- aclf.write('group admins b...@qpid \ ')
+ aclf.write('group admins b...@qpid \n')
aclf.write(' \ \n')
aclf.write('j...@qpid \n')
aclf.write('acl allow all all')
aclf.close()
result = self.reload_acl()
- if (result.text.find("contains illegal characters",0,len(result.text))
== -1):
+ if (result.text.find("contains an illegal
extension",0,len(result.text)) == -1):
+ self.fail(result)
+
+ if (result.text.find("Non-continuation line must start with \"group\"
or \"acl\"",0,len(result.text)) == -1):
self.fail(result)
+
def test_user_domain(self):
"""
Test a user defined without a realm
Ex. group admin rajith
"""
- aclf = ACLFile()
- aclf.write('group test j...@example.com\n') # should be allowed
+ aclf = ACLFile()
aclf.write('group admin bob\n') # shouldn't be allowed
aclf.write('acl deny admin bind exchange\n')
aclf.write('acl allow all all')
@@ -211,6 +184,23 @@ class ACLTests(TestBase010):
if (result.text.find("Username 'bob' must contain a
realm",0,len(result.text)) == -1):
self.fail(result)
+ def test_allowed_chars_for_username(self):
+ """
+ Test a user defined without a realm
+ Ex. group admin rajith
+ """
+ aclf = ACLFile()
+ aclf.write('group test1 j...@example.com\n') # should be allowed
+ aclf.write('group test2 jack-j...@example.com\n') # should be allowed
+ aclf.write('group test3 jack_j...@example.com\n') # should be allowed
+ aclf.write('group test4 host/somemachine.example....@example.com\n') #
should be allowed
+ aclf.write('acl allow all all')
+ aclf.close()
+
+ result = self.reload_acl()
+ if (result.text.find("ACL format error",0,len(result.text)) != -1):
+ self.fail(result)
+
#=====================================
# ACL validation tests
#=====================================
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:commits-subscr...@qpid.apache.org
