Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PS-2099] PostgreSQL EF Contains() in SQL Query is strpos(), witch is case-sens… #2512

Merged
merged 6 commits into from
Jan 19, 2023

Conversation

juliokele
Copy link
Contributor

After Items importing the Cipher/Folders value is uppercase.
PostgreSQL EF Contains() under the hood in SQL Query is strpos(), witch is case-sensitive.

@CLAassistant
Copy link

CLAassistant commented Dec 21, 2022

CLA assistant check
All committers have signed the CLA.

@bitwarden-bot
Copy link

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PS-2099

@bitwarden-bot bitwarden-bot changed the title PostgreSQL EF Contains() in SQL Query is strpos(), witch is case-sens… [PS-2099] PostgreSQL EF Contains() in SQL Query is strpos(), witch is case-sens… Dec 21, 2022
@djsmith85 djsmith85 added the bw-unified-deploy An Issue related to Bitwarden unified deployment label Dec 21, 2022
@justindbaur
Copy link
Member

Hi @juliokele thank you so much for this contribution. Did you by chance test this against MySQL to make sure this solution still allowed it to work properly?

@justindbaur justindbaur self-assigned this Dec 21, 2022
@juliokele
Copy link
Contributor Author

I have tested against MySQL and working properly.

@@ -28,7 +28,7 @@ public virtual IQueryable<CipherDetails> Run(DatabaseContext dbContext)
DeletedDate = c.DeletedDate,
Reprompt = c.Reprompt,
Favorite = _userId.HasValue && c.Favorites != null && c.Favorites.Contains($"\"{_userId}\":true"),
Copy link
Member

Choose a reason for hiding this comment

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

Dont we have this same problem here too? I think we do these kinds of checks in some other areas too. @justindbaur UserCipherDetailsQuery maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the same problem (Favorite is upper-case too):

cipher.Favorites = $"{{\"{cipher.UserId.ToString().ToUpperInvariant()}\":\"true\"}}";

@@ -28,7 +28,7 @@ public virtual IQueryable<CipherDetails> Run(DatabaseContext dbContext)
DeletedDate = c.DeletedDate,
Reprompt = c.Reprompt,
Favorite = _userId.HasValue && c.Favorites != null && c.Favorites.Contains($"\"{_userId}\":true"),
FolderId = (_ignoreFolders || !_userId.HasValue || c.Folders == null || !c.Folders.Contains(_userId.Value.ToString())) ?
FolderId = (_ignoreFolders || !_userId.HasValue || c.Folders == null || !c.Folders.ToLowerInvariant().Contains(_userId.Value.ToString())) ?
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to ensure _userId is lowercase as well?

@juliokele juliokele closed this by deleting the head repository Jan 9, 2023
@kspearrin kspearrin reopened this Jan 9, 2023
@juliokele
Copy link
Contributor Author

@kspearrin
Sorry for deleting my head repository before PR merging!

@kspearrin
Copy link
Member

@juliokele no worries, we're trying to figure out some issues with unsigned commits before being able to merge.

@juliokele juliokele mentioned this pull request Jan 13, 2023
1 task
@kspearrin
Copy link
Member

@justindbaur Should we merge this for next release?

@justindbaur
Copy link
Member

@kspearrin Yes we should, have we figured out the unsigned commit issue?

@kspearrin kspearrin merged commit 9e75f65 into bitwarden:master Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bw-unified-deploy An Issue related to Bitwarden unified deployment community-pr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After Data import, Item binding to Folder missing (only with PostgreSQL Database)
6 participants