You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lenya.apache.org by Joern Nettingsmeier <po...@uni-due.de> on 2006/08/31 20:07:33 UTC
Re: svn commit: r438951 - /lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java
thorsten@apache.org wrote:
> Author: thorsten
> Date: Thu Aug 31 08:29:07 2006
> New Revision: 438951
>
> URL: http://svn.apache.org/viewvc?rev=438951&view=rev
> Log:
> Fixing bug for adding a role to a policy that was introduced in -r438936.
> Thanks to Josias for testing and spotting the bug.
>
> Modified:
> lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java
>
> Modified: lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java
> URL: http://svn.apache.org/viewvc/lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java?rev=438951&r1=438950&r2=438951&view=diff
> ==============================================================================
> --- lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java (original)
> +++ lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java Thu Aug 31 08:29:07 2006
> @@ -240,38 +243,41 @@
> */
> protected void removeCredential(Credential credential)
> throws AccessControlException {
> - String key = credential.getAccreditable().toString() + KEY_SEPERATOR
> - + credential.getRoles()[0];
> - if (this.accreditableToCredential.containsKey(key)) {
> - Credential[] oldCredentials = (Credential[]) this.accreditableToCredential
> - .get(key);
> - int occurrence;
> - boolean out = false;
> - for (occurrence = 0; occurrence < oldCredentials.length; occurrence++) {
> - Credential current = oldCredentials[occurrence];
> - Role[] checkRole = current.getRoles();
> - for (int j = 0; j < checkRole.length; j++) {
> - Role role2 = checkRole[j];
> - if (role2.equals(credential.getRoles()[0])) {
> - out = true;
> - break;
> + if (credential.getRoles().length > 0) {
> + String key = credential.getAccreditable().toString()
> + + KEY_SEPERATOR + credential.getRoles()[0];
> + if (this.accreditableToCredential.containsKey(key)) {
> + Credential[] oldCredentials = (Credential[]) this.accreditableToCredential
> + .get(key);
> + int occurrence;
> + boolean out = false;
> + for (occurrence = 0; occurrence < oldCredentials.length; occurrence++) {
> + Credential current = oldCredentials[occurrence];
> + Role[] checkRole = current.getRoles();
> + for (int j = 0; j < checkRole.length; j++) {
> + Role role2 = checkRole[j];
> + if (role2.equals(credential.getRoles()[0])) {
> + out = true;
> + break;
> + }
> }
> + if (out)
> + break;
> }
> - if (out)
> - break;
> - }
why not this?
check_credentials:
for (...) {
for (...) {
for (...) {
break check_credentials;
}
}
}
sorry for being such a nuisance, but i find this code very hard to
understand in many places. convoluted structure and really strange naming.
for instance, checkRole sounds like a boolean function to me - least of
all i would expect it to be an array of roles. how about "rolesToCheck"
instead?
or "occurrence": to my mind, it is a counter that gets incremented when
something special occurs. for a simple loop index, why not use the tried
and tested "i"?
what is "role2"? obviously it's one role of the array of roles to be
checked... so why not "roleToCheck", or just "currentRole"? role2
implies that there is also a role1 and these are somehow used together.
since this is highly security-relevant code, it should be very carefully
written (i.e. so that idiots like me can understand it with half of
their brains on stand-by, and no, that's not egotism on my part, that's
a basic design paradigm when security is concerned).
the code must be easily audited and its workings should be made obvious.
this is not currently the case imho.
--
"I don't need backups. I need restore!" - Trad.
--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: svn commit: r438951 - /lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java
Posted by Jörn Nettingsmeier <po...@uni-duisburg.de>.
Thorsten Scherler wrote:
> El jue, 31-08-2006 a las 20:07 +0200, Joern Nettingsmeier escribió:
> ...
>> why not this?
>>
>> check_credentials:
>> for (...) {
>> for (...) {
>> for (...) {
>> break check_credentials;
>> }
>> }
>> }
>>
>
> ...because it would not work.
it does here:
nettings@kleineronkel:~> cat test.java
public class test {
public static void main(String[] argv) {
nested_loop:
for (int i=0; i<5; ++i) {
for (int k=0; k<5; ++k) {
if (i == 0 && k == 1) break nested_loop;
System.out.println("i: " + i + "\tk: " + k);
}
}
}
}
nettings@kleineronkel:~> java test
i: 0 k: 0
>> sorry for being such a nuisance, but i find this code very hard to
>> understand in many places. convoluted structure and really strange naming.
>
> patches welcome
i'll try and test the code early next week, and provide patches if i can.
thorsten, i hope i have not offended you personally. it's just that i've
been on the receiving end of a lot of happy-go-lucky coding lately (not
only lenya-related), plus i may have read too much of lkml to be let
loose among civilised dev communities...
best,
jörn
--
"Open source takes the bullshit out of software."
- Charles Ferguson on TechnologyReview.com
--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: pol-admin@uni-due.de, Telefon: 0203/379-2736
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: svn commit: r438951 -
/lenya/sandbox/ac-restricted-1.4-src/modules-core/ac-impl/java/src/org/apache/lenya/ac/impl/DefaultPolicy.java
Posted by Thorsten Scherler <th...@wyona.com>.
El jue, 31-08-2006 a las 20:07 +0200, Joern Nettingsmeier escribió:
...
>
> why not this?
>
> check_credentials:
> for (...) {
> for (...) {
> for (...) {
> break check_credentials;
> }
> }
> }
>
...because it would not work.
>
> sorry for being such a nuisance, but i find this code very hard to
> understand in many places. convoluted structure and really strange naming.
>
patches welcome
salu2
--
Thorsten Scherler
COO Spain
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
thorsten.scherler@wyona.com thorsten@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org