You've already forked AstralRinth
Tighten URL slug validation (#6442)
* Tighten URL slug validation * slug sanitization in frontend
This commit is contained in:
@@ -89,6 +89,8 @@ import {
|
||||
} from '@modrinth/ui'
|
||||
import { ref } from 'vue'
|
||||
|
||||
import { generateUrlSlug } from '~/utils/slugs'
|
||||
|
||||
import CreateLimitAlert from './CreateLimitAlert.vue'
|
||||
|
||||
const router = useNativeRouter()
|
||||
@@ -148,7 +150,7 @@ async function createOrganization(): Promise<void> {
|
||||
const value = {
|
||||
name: name.value.trim(),
|
||||
description: description.value.trim(),
|
||||
slug: slug.value.trim().replace(/ +/g, ''),
|
||||
slug: slug.value.trim(),
|
||||
}
|
||||
|
||||
const result: any = await useBaseFetch('organization', {
|
||||
@@ -183,12 +185,7 @@ function hide(): void {
|
||||
|
||||
function updateSlug(): void {
|
||||
if (!manualSlug.value) {
|
||||
slug.value = name.value
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.replaceAll(' ', '-')
|
||||
.replaceAll(/[^a-zA-Z0-9!@$()`.+,_"-]/g, '')
|
||||
.replaceAll(/--+/gm, '-')
|
||||
slug.value = generateUrlSlug(name.value)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -145,6 +145,8 @@ import {
|
||||
} from '@modrinth/ui'
|
||||
import { computed, defineAsyncComponent, h } from 'vue'
|
||||
|
||||
import { generateUrlSlug } from '~/utils/slugs'
|
||||
|
||||
import CreateLimitAlert from './CreateLimitAlert.vue'
|
||||
|
||||
type ProjectTypes = 'server' | 'project'
|
||||
@@ -461,12 +463,7 @@ async function show(event?: MouseEvent, options?: ShowOptions) {
|
||||
|
||||
function updatedName() {
|
||||
if (!manualSlug.value) {
|
||||
slug.value = name.value
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.replaceAll(' ', '-')
|
||||
.replaceAll(/[^a-zA-Z0-9!@$()`.+,_"-]/g, '')
|
||||
.replaceAll(/--+/gm, '-')
|
||||
slug.value = generateUrlSlug(name.value)
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
const PROJECT_SLUG_UNSAFE_CHARS = /[^a-zA-Z0-9._-]/g
|
||||
|
||||
export function generateUrlSlug(value: string) {
|
||||
return value
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.replaceAll(' ', '-')
|
||||
.replaceAll(PROJECT_SLUG_UNSAFE_CHARS, '')
|
||||
.replaceAll(/--+/gm, '-')
|
||||
}
|
||||
@@ -184,14 +184,14 @@ pub async fn projects_list(
|
||||
|
||||
#[derive(Serialize, Deserialize, Validate, utoipa::ToSchema)]
|
||||
pub struct EditUser {
|
||||
#[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_USERNAME))]
|
||||
#[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_URL_SAFE))]
|
||||
pub username: Option<String>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "::serde_with::rust::double_option"
|
||||
)]
|
||||
#[validate(length(min = 1, max = 64), regex(path = *crate::util::validate::RE_USERNAME))]
|
||||
#[validate(length(min = 1, max = 64), regex(path = *crate::util::validate::RE_URL_SAFE))]
|
||||
pub name: Option<Option<String>>,
|
||||
#[serde(
|
||||
default,
|
||||
|
||||
@@ -36,7 +36,7 @@ pub struct InitialVersionData {
|
||||
pub file_parts: Vec<String>,
|
||||
#[validate(
|
||||
length(min = 1, max = 32),
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE)
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED)
|
||||
)]
|
||||
pub version_number: String,
|
||||
#[validate(
|
||||
|
||||
@@ -314,7 +314,7 @@ pub struct EditVersion {
|
||||
pub name: Option<String>,
|
||||
#[validate(
|
||||
length(min = 1, max = 32),
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE)
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED)
|
||||
)]
|
||||
pub version_number: Option<String>,
|
||||
#[validate(length(max = 65536))]
|
||||
|
||||
@@ -101,6 +101,7 @@ impl ResponseError for CreateError {
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Validate, utoipa::ToSchema)]
|
||||
pub struct ProjectCreate {
|
||||
#[validate(nested)]
|
||||
pub base: exp::base::Project,
|
||||
#[serde(flatten)]
|
||||
#[validate(nested)]
|
||||
@@ -338,3 +339,59 @@ pub async fn create(
|
||||
|
||||
Ok(web::Json(project_id))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::models::projects::ProjectStatus;
|
||||
|
||||
fn project_create_with_slug(slug: &str) -> ProjectCreate {
|
||||
ProjectCreate {
|
||||
base: exp::base::Project {
|
||||
name: "test project".into(),
|
||||
slug: slug.into(),
|
||||
summary: "test summary".into(),
|
||||
description: String::new(),
|
||||
requested_status: ProjectStatus::Approved,
|
||||
organization_id: None,
|
||||
},
|
||||
components: exp::ProjectEdit {
|
||||
minecraft_mod: None,
|
||||
minecraft_server: None,
|
||||
minecraft_java_server: None,
|
||||
minecraft_bedrock_server: None,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn assert_project_slug_validation(slug: &str, expected_valid: bool) {
|
||||
let result = project_create_with_slug(slug).validate();
|
||||
|
||||
assert_eq!(
|
||||
result.is_ok(),
|
||||
expected_valid,
|
||||
"unexpected validation result for slug `{slug}`"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_create_accepts_url_safe_base_slugs() {
|
||||
for slug in ["valid-slug", "valid_slug", "valid.slug", "valid123"] {
|
||||
assert_project_slug_validation(slug, true);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_create_rejects_unsafe_base_slugs() {
|
||||
for slug in [
|
||||
"invalid/slug",
|
||||
"../invalid",
|
||||
r#"invalid"slug"#,
|
||||
"invalid$slug",
|
||||
"invalid slug",
|
||||
"invalid#slug",
|
||||
] {
|
||||
assert_project_slug_validation(slug, false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -642,7 +642,7 @@ pub async fn orgs_list(
|
||||
|
||||
#[derive(Serialize, Deserialize, Validate)]
|
||||
pub struct EditUser {
|
||||
#[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_USERNAME))]
|
||||
#[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_URL_SAFE))]
|
||||
pub username: Option<String>,
|
||||
#[serde(
|
||||
default,
|
||||
|
||||
@@ -56,7 +56,7 @@ pub struct InitialVersionData {
|
||||
pub file_parts: Vec<String>,
|
||||
#[validate(
|
||||
length(min = 1, max = 32),
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE)
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED)
|
||||
)]
|
||||
pub version_number: String,
|
||||
#[validate(
|
||||
|
||||
@@ -222,7 +222,7 @@ pub struct EditVersion {
|
||||
pub name: Option<String>,
|
||||
#[validate(
|
||||
length(min = 1, max = 32),
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE)
|
||||
regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED)
|
||||
)]
|
||||
pub version_number: Option<String>,
|
||||
#[validate(length(max = 65536))]
|
||||
|
||||
@@ -7,9 +7,12 @@ use validator::{ValidationErrors, ValidationErrorsKind};
|
||||
use crate::models::pats::Scopes;
|
||||
|
||||
pub static RE_URL_SAFE: LazyLock<Regex> =
|
||||
LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9!@$()`.+,_"-]*$"#).unwrap());
|
||||
pub static RE_USERNAME: LazyLock<Regex> =
|
||||
LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9_-]*$"#).unwrap());
|
||||
LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9._-]+$"#).unwrap());
|
||||
|
||||
// only used for versions
|
||||
// TODO: percent-encode version names in URLs instead of treating them as slugs
|
||||
pub static RE_URL_SAFE_RELAXED: LazyLock<Regex> =
|
||||
LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9!@$()`.+,_"-]+$"#).unwrap());
|
||||
|
||||
//TODO: In order to ensure readability, only the first error is printed, this may need to be expanded on in the future!
|
||||
pub fn validation_errors_to_string(
|
||||
@@ -159,4 +162,70 @@ mod tests {
|
||||
let result = validate_name(" ");
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
fn assert_url_safe_regex(
|
||||
regex: &LazyLock<Regex>,
|
||||
value: &str,
|
||||
expected_valid: bool,
|
||||
) {
|
||||
assert_eq!(
|
||||
regex.is_match(value),
|
||||
expected_valid,
|
||||
"unexpected URL-safe validation result for `{value}`"
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_url_safe_slug(slug: &str, expected_valid: bool) {
|
||||
assert_url_safe_regex(&RE_URL_SAFE, slug, expected_valid);
|
||||
}
|
||||
|
||||
fn assert_url_safe_version(version: &str, expected_valid: bool) {
|
||||
assert_url_safe_regex(&RE_URL_SAFE_RELAXED, version, expected_valid);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn url_safe_regex_accepts_allowed_slug_punctuation() {
|
||||
for slug in ["valid-slug", "valid_slug", "valid.slug", "valid123"] {
|
||||
assert_url_safe_slug(slug, true);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn url_safe_regex_rejects_unsafe_slug_punctuation() {
|
||||
for slug in [
|
||||
"invalid/slug",
|
||||
"../invalid",
|
||||
r#"invalid"slug"#,
|
||||
"invalid$slug",
|
||||
"invalid slug",
|
||||
"invalid#slug",
|
||||
] {
|
||||
assert_url_safe_slug(slug, false);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn url_safe_relaxed_regex_accepts_legacy_version_punctuation() {
|
||||
for version in [
|
||||
"1.0.0",
|
||||
"1.0.0+build",
|
||||
"version$beta",
|
||||
r#"version"quoted"#,
|
||||
"version!@$()`.+,_-",
|
||||
] {
|
||||
assert_url_safe_version(version, true);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn url_safe_relaxed_regex_rejects_non_version_safe_punctuation() {
|
||||
for version in [
|
||||
"invalid/version",
|
||||
"../invalid",
|
||||
"invalid space",
|
||||
"invalid#version",
|
||||
] {
|
||||
assert_url_safe_version(version, false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user