Restrict characters in URLs of packages and platforms #6962 #7730
Conversation
| if let Some((index, ch)) = url | ||
| .chars() | ||
| .any(|ch| MISLEADING_CHARACTERS_IN_URL.contains(&ch)) | ||
| .enumerate() | ||
| .find(|(_, ch)| (!ch.is_alphanumeric() && !ALLOWED_URL_CHARACTERS.contains(ch)) || MISLEADING_CHARACTERS_IN_URL.contains(ch)) | ||
| { | ||
| return Err(UrlProblem::MisleadingCharacter); | ||
| // Check if there is an intentionally misleading url character | ||
| // Otherwise check if there is a possibly insecure character in the url | ||
| if MISLEADING_CHARACTERS_IN_URL.contains(&ch) { | ||
| return Err(UrlProblem::MisleadingCharacter); | ||
| } else { | ||
| return Err(UrlProblem::InsecureCharacter((ch.to_string(), index))); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
I think this'd be a bit easier to read/understand if rather than making this a single if, you split it out into two: one that's exactly like what was there before, and a second one after that to check this new condition.
|
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
Added parser functionality to throw an error if disallowed URL characters appear to mitigate security issues through platforms and package url's