search

rust-lang / crates.io

The Rust package registry
https://crates.io
Apache License 2.0
2.95k stars 598 forks source link

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

Open carols10cents opened 2 years ago

carols10cents commented 2 years ago

This may be related to https://github.com/rust-lang/crates.io/issues/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....

Turbo87 commented 2 years ago

I assume this is the same as https://github.com/rust-lang/crates.io/issues/4252?

carols10cents commented 2 years ago

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

Bogay commented 2 years ago

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 commented 2 years ago

@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 commented 1 year ago

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.