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

Deleting a team results in a 500 because of a uniqueness constraint violation #4281

Open
carols10cents opened this issue Dec 16, 2021 · 5 comments
Labels
A-backend ⚙️ A-teams C-bug 🐞 Category: unintended, undesired behavior

Comments

@carols10cents
Copy link
Member

carols10cents commented Dec 16, 2021

This may be related to #1818

Someone reported via email that they tried to remove a team as an owner of a crate. The team existed on GitHub, as far as I can tell. The command they ran was:

cargo owner --remove github:OrgName:TeamName

crates.io returned a 500, and I see this in the logs (irrelevant details removed):

 Dec 13 14:56:32 app/web.4 at=error method=DELETE path="crates/[crate name]/owners" status=500 user_agent="cargo 1.57.0 (b2e52d7ca 2021-10-21)" error="duplicate key value violates unique constraint "teams_login_key""

I'm not sure why a DELETE owners request is trying to... update? insert? into the teams table such that the unique constraint is violated. I haven't looked into the code yet, and I'm going to fix the database for this user.

My current suspicion is that this has something to do with case sensitivity-- in the database, we have the team login stored as github:orgname:teamname but the user specified github:OrgName:TeamName to cargo....

@carols10cents carols10cents added A-teams C-bug 🐞 Category: unintended, undesired behavior labels Dec 16, 2021
@Turbo87
Copy link
Member

Turbo87 commented Dec 16, 2021

I assume this is the same as #4252?

@carols10cents
Copy link
Member Author

carols10cents commented Dec 16, 2021

Yup, I think you're right. Closed that one because I put more detail here.

@Bogay
Copy link

Bogay commented Oct 12, 2022

Hello, I want to work on this and have taken a look at the code.
Because the GitHub API doc says that parameters are case insensitive. I think casting user input to lowercase before sending the request to GitHub will solve this issue.
Can I open a PR for that?

圖片

@Turbo87
Copy link
Member

Turbo87 commented Oct 17, 2022

@Bogay "case insensitive" does not mean that GitHub needs a lowercase string, it means that they don't care whether it's lowercase, UPPERCASE or AnyTHIng nI beTWeen.

the issue is rather that crates.io itself appears to care about the casing, before we even send the request to GitHub.

@nic-hartley
Copy link
Contributor

As far as I can tell, we're always storing the lowercased team name currently: src/models/team.rs:161 is where GitHub Teams get added to the database, and it stores it as login.to_lowercase(). I'm not sure this is still an issue, though existing teams might have the wrong casing still in the database, and we might need an UPDATE teams SET login = LOWER(login). Or that might have been done sometime in the interim and this isn't actually an issue anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ A-teams C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

No branches or pull requests

4 participants