Skip to content

Conversation

elrido
Copy link
Contributor

@elrido elrido commented Jul 7, 2024

This PR fixes #1361.

Changes

  • Simpler PostgreSQL table lookup query, doesn't require permission to read the user table

Note that while I did validate the query on a PostgreSQL 16.1 docker container, we have no PG unit tests and I have no PrivateBin + Postgres setup available to test this on. I would appreciate if someone with a PG setup could test this branch and let us know if this causes any problems or if there would be a better/more robust/simpler query.

Copy link
Member

@rugk rugk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uff this is really a crazy query and hard to review. Again I also have no real Postgres experience, but this surely has potential to optimize. Also given this is just a "show table", this should be a solved problem, is not it? Why was/is it so complex?

You find plenty of Stackoverflow for it: https://stackoverflow.com/questions/2276644/list-all-tables-in-postgresql-information-schema

The new thing certainly looks fine, the old thing looks crazy… (especially if you compare it to the other DBs 😆 )

Maybe also use ChatGPT or some other AI thiny to check the query or leave it out and generate one?

Obviously why should I be opposed but @Lowaiz can likely best say whether that's okay.

@Lowaiz
Copy link

Lowaiz commented Jul 8, 2024

We are running PrivateBin with the docker image privatebin/pdo so I can't try easily this update.
If you can release a rc-patch or a temporary image, I'll test it right away.
But as I mentioned in the issue, the query was fine.

@Lowaiz
Copy link

Lowaiz commented Jul 8, 2024

From GitHub Copilot Chat:

Yes, the PostgreSQL query you've selected is correct for listing table names that are not part of the system catalogs (pg_catalog and information_schema). This query filters out system tables and returns only the user-defined tables in the database.

SELECT "tablename" FROM "pg_catalog"."pg_tables" 
WHERE "schemaname" NOT IN ('pg_catalog', 'information_schema');

This approach ensures that the query results include only the tables created by users, which is typically what applications need to interact with.

@elrido elrido merged commit cf95e0b into master Jul 9, 2024
@elrido elrido deleted the pg-tables-query branch July 9, 2024 19:30
@Lowaiz
Copy link

Lowaiz commented Jul 15, 2024

FYI, I tested the new query by overriding the Database.php file in a copy of the image and everything runs smoothly.

@Lowaiz
Copy link

Lowaiz commented Jul 31, 2024

Just for my information, when do you plan to release a version including this fix ?

@elrido
Copy link
Contributor Author

elrido commented Aug 1, 2024

There are currently no plans. We had to rush the last release a little, due to the security fix. So far this is the only item up for the next release. Holiday seasons is still ongoing and mine is still coming up. Depending on activities in September and October, I'd assume we have a few things warranting a release by late fall, maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pgsql: crash when serice account do not have SELECT right on pg_catalog.pg_user
3 participants