Who needs regular expressions anyway?
-
<%
function CheckAuth(moduleId, accessType)
Set Data = New DBData.open Data.OpenGlobal Data.runQuery("SELECT * FROM pages ORDER BY id DESC") if not moduleId = 0 then cAccess = Session("level") AccessList = Split(cAccess,", ") marker = 1 for each i in AccessList if isNumeric(Mid(i,marker,1)) then string1 = Mid(i,marker,1) marker = marker + 1 if isNumeric(Mid(i,marker,1)) then string1 = string1 & Mid(i,marker,1) marker = marker + 1 end if end if marker = marker + 1 if Mid(i,marker,1) = "A" then Execute("auth" & string1 & "a = 1") marker = marker + 1 else Execute("auth" & string1 & "a = 0") marker = marker + 1 end if if Mid(i,marker,1) = "E" then Execute("auth" & string1 & "e = 1") marker = marker + 1 else Execute("auth" & string1 & "e = 0") marker = marker + 1 end if if Mid(i,marker,1) = "D" then Execute("auth" & string1 & "d = 1") marker = marker + 1 else Execute("auth" & string1 & "d = 0") marker = marker + 1 end if marker = 1 Next n = 1 Execute("test = auth" & moduleId & accessType) if test = 0 then AuthShow = false else AuthShow = true end if end ifend function
%>
Now let's explain what's going on here. When a user logs in, their access control info is taken from a single field in the database and stored in the sesion variable called "access" (anonymized, of course). The contents of this field/variable are of the form "1-AED, 2-A--, 3--E-", where the number is a "module" ID (that is, some part of the system) and then add/edit/delete privileges for that item. This method takes the id (the numeric portion) and a single letter, a e or d. It then tells you wether or not it is a match for the current user's privileges.
This is from a real, live, multi-million dollar system developed for a major branch of the US gov't. Find the WTFs. There are at least a dozen... and they're [i]good[/i].
-
Ooh ooh I spotted one! Only supports 2-digit modules!
Oh there's another! The A, E, and D must be in that order for them to work properly, and must contain some kind of placeholder if the permission hasn't been granted. Most variations will render the permission useless, like "1-DAE", or "2-ED".
#3 - If the first character in the array value isn't a number, it will use the module ID from the previous array entry.
Aside from those, it's just crap coding all around, something you'd throw together to manage your personal MS Access db that you use to manage your checkbook. Not what you should expect in a government-run system. Sadly it's what you are more likely to see these days.
-
@Manni said:
Ooh ooh I spotted one! Only supports 2-digit modules!
Oh there's another! The A, E, and D must be in that order for them to work properly, and must contain some kind of placeholder if the permission hasn't been granted. Most variations will render the permission useless, like "1-DAE", or "2-ED".
#3 - If the first character in the array value isn't a number, it will use the module ID from the previous array entry.
Aside from those, it's just crap coding all around, something you'd throw together to manage your personal MS Access db that you use to manage your checkbook. Not what you should expect in a government-run system. Sadly it's what you are more likely to see these days.
Right on about the 2-digit number limit. Also, there is usually a "-" placeholder so that an entry would be "1--E-" if there was no A or D on it.
Now here's a clue... look carefully at the database query and how it's used.
-
I love all the marker = marker + 1 ...
-
WTF?
- The query is ignored.
- Everything in the access list is enumerated, calling execute(!) to SET a variable for each module ID and access type(!!).
- Finally, execute is used again to GET the value of the appropriate variable.
- Not even getting into AuthShow global...
-
@samael said:
WTF?
- The query is ignored.
- Everything in the access list is enumerated, calling execute(!) to SET a variable for each module ID and access type(!!).
- Finally, execute is used again to GET the value of the appropriate variable.
- Not even getting into AuthShow global...
Oops... the "AuthShow" is the original name of the method, and I missed it in my lame attempt to anonymize it. That's the return statement.
-
I have never used Execute, so I'm not exactly sure what it does. Am I correct that in this case it auto-creates 3 variables for each module?
I'm now at the point where I think this is the worst piece of code ever written, and we are all dumber for having read it. He processes every permission level in every module just to confirm or deny one permission in one module. He does this random variable creation bullshit, setting them to 1 or 0 based on if they're true or false, then sets another variable at the end to 1 or 0 based on the one he's looking for, then ... GYAR I think something in my brain just popped.
Someone restore some sanity to my life. Code this in as few lines as possible or I'll be forced to just so I can continue with my day.
-
@Manni said:
I have never used Execute, so I'm not exactly sure what it does. Am I correct that in this case it auto-creates 3 variables for each module?
I'm now at the point where I think this is the worst piece of code ever written, and we are all dumber for having read it. He processes every permission level in every module just to confirm or deny one permission in one module. He does this random variable creation bullshit, setting them to 1 or 0 based on if they're true or false, then sets another variable at the end to 1 or 0 based on the one he's looking for, then ... GYAR I think something in my brain just popped.
Someone restore some sanity to my life. Code this in as few lines as possible or I'll be forced to just so I can continue with my day.
Well that's going a little overboard. To be fair the project was dumped on a guy who wasn't ready for it several years ago. He's now totally up to speed. Anyway, here's the short version:
function CheckAuth(moduleID, accessType) set AuthMatcher = new RegExp AuthMatcher.IgnoreCase = True AuthMatcher.Pattern = "(^| |,)" & moduleID & "-[\-\w]*" & accessType & "[\-\w]*(,|$)" CheckAuth = AuthMatcher.Test(Trim(Session("level"))) end function
-
[quote user="djork"]Well that's going a little overboard.[/quote]
...fair enough. I've been known to be overly-critical when the WTFs start piling up. I'm sure there's way worse code out there, and I was exaggerating when I said something popped in my brain. It was closer to the groinal region.
[quote user="djork"]
function CheckAuth(moduleID, accessType) set AuthMatcher = new RegExp AuthMatcher.IgnoreCase = True AuthMatcher.Pattern = "(^| |,)" & moduleID & "-[\-\w]*" & accessType & "[\-\w]*(,|$)" CheckAuth = AuthMatcher.Test(Trim(Session("level"))) end function[/quote]
Beautiful. I would bring up VB's slight lag when instantiating a new RegExp object, but since you've replaced an unnecessary db query and an over-complicated For loop, it's probably still a great performance improvement.
-
No one's mentioned the retarded database structure? The 'formatted data string in a VARCHAR' pattern is its own WTF.
-
@merreborn said:
No one's mentioned the retarded database structure? The 'formatted data string in a VARCHAR' pattern is its own WTF.
That is nothing compared to some other DB WTFs. Because a certain field is concatenated where data should be separated we have glorious lines of code like this:
Loop Until (Left(DataArray(0, R), Len(DataArray(0, R)) - 4) <> Left(DataArray(0, R + 1), Len(DataArray(0, R + 1)) - 4))
-
[quote user="accident"]I love all the marker = marker + 1 ...[/quote]
VB didn't include marker += 1 until VS.NET 2003. That's probably what this is.
It still doesn't accept the increment / decrement operators.
-
[quote user="themagni"]
[quote user="accident"]I love all the marker = marker + 1 ...[/quote]
VB didn't include marker += 1 until VS.NET 2003. That's probably what this is.
It still doesn't accept the increment / decrement operators.
[/quote]
I don't think that's what he meant. I took it more to mean "Look at how many times it's unnecessarily repeated".
Like it's included twice in each If block, when really it only needs to be there once. And if the coder is expecting A to be first, E to be second, etc, then that variable isn't really needed at all.
-
Nevermind...
-
[quote user="Manni"]
and we are all dumber for having read it.
[/quote]
W00t!!! I didn't read it, so I don't lose any smartness points.
