DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
8 stars 2 forks source link

User names are forcibly made case sensitive (W) #86

Closed smurthys closed 7 years ago

smurthys commented 7 years ago

Function createUserformats user name with %I which forces user names to be case sensitive in all future operations. This behavior is contrary to expectations of users who invokecreateUser and other similar functions without double quotes around user names. This behavior can also cause functions such listUserConnections to fail in some cases.

afig commented 7 years ago

Using %s does result in the desired behavior. Running the following statements creates the users shown

createStudent shown, but other createXYZ have the same behavior after they have been switch to using %s:

SELECT classdb.createStudent('lctest', 'noname');
SELECT classdb.createStudent('UCtest', 'noname');
SELECT classdb.createStudent('"QTUtest"', 'noname');
SELECT classdb.createStudent('"qtltest"', 'noname');

SELECT rolname FROM pg_roles;

      rolname
--------------------
... 
 lctest
 uctest
 QTUtest
 qtltest

Rows omitted in the resulting table

To verify case-sensitivity (or lack thereof)

ALTER ROLE lctest;
    ALTER ROLE
ALTER ROLE Lctest;
    ALTER ROLE
ALTER ROLE "lctest";
    ALTER ROLE
ALTER ROLE "LCtest";
    ERROR:  role "LCtest" does not exist

ALTER ROLE uctest;
    ALTER ROLE
ALTER ROLE UCtest;
    ALTER ROLE
ALTER ROLE "uctest";
    ALTER ROLE
ALTER ROLE "UCtest";
    ERROR:  role "UCtest" does not exist

ALTER ROLE qtutest;
    ERROR:  role "qtutest" does not exist
ALTER ROLE QTUtest;
    ERROR:  role "qtutest" does not exist
ALTER ROLE "qtutest";
    ERROR:  role "qtutest" does not exist
ALTER ROLE "QTUtest"
    ALTER ROLE

ALTER ROLE qtltest;
    ALTER ROLE
ALTER ROLE Qtltest;
    ALTER ROLE
ALTER ROLE "qtltest";
    ALTER ROLE
ALTER ROLE "Qtltest";
    ERROR:  role "Qtltest" does not exist

However, an issue persists: attempting to match a role name to an entry in pg_roles is always case-sensitive. This results in an issue whenever a check for a role's existence is performed. I will investigate fixes for this issue.

afig commented 7 years ago

The only feasible solution I could find is: If it the value of the parameter is not enclosed in quotes, call the lower function on it. This will work in all "reasonable" situations. I say "reasonable" situations because it would be possible to call the function with a role name such as 'ab"cd"ef'. However, in that situation, the function would have resulted in a syntax error anyways. This solution is a bit messy to implement in every situation where it is necessary. Therefore, it would likely be best implemented through a "helper" function.

Through the process of finding a solution, I learned that PostgreSQL allows for a lot of leeway in terms of allowed characters in quoted identifiers. As a matter of fact, it turns out that if you quote a role name, you can have any character* (except for \0 ). Even a new line character.

Another thing of note is that in psql, usernames are always case-sensitive, even for a "normal" case. This makes sense, and is likely the case for all Postgres clients. Otherwise, a situation could arise where the server would not know which of two role names to pick, if the two names only differed by case.

* I have not verified this, but the characters allowed would likely be any that are in the current locale for the DB Cluster

smurthys commented 7 years ago

Thank you for the detailed analysis. I think it will work to add a helper function for pg_roles as you describe.

BTW, one of us can create a separate issue if necessary, but as I mentioned this morning, %I is used to format even schema and DB names. I wonder if either/both of those cause the same problem as user name does.

afig commented 7 years ago

Using %I should not be an issue where we're using current_database, since %I essentially forces case-sensitivity, therefore, it's mainly an issue during object creation, or when relying on the folding of identifiers. If the database were to be named 'dassl', then %I will leave the identifier as dassl, however, if the database was named DASSL, the %I will add quotes, making it "DASSL".

The use of %I during the creation of schemas had also been removed, along with its use when assigning privileges.

smurthys commented 7 years ago

Thank you. I now recall discussing earlier today that %I is not an issue when used with current_database.