changes from self review

This commit is contained in:
Georges-Antoine Assi
2026-03-07 09:36:45 -05:00
parent b3659a1226
commit 76bdfb4891
9 changed files with 95 additions and 144 deletions

View File

@@ -31,7 +31,7 @@ from models.collection import (
VirtualCollection,
)
from utils.router import APIRouter
from utils.validation import ValidationError, validate_url_for_http_request
from utils.validation import ValidationError
router = APIRouter(
prefix="/collections",
@@ -70,14 +70,6 @@ async def add_collection(
"user_id": request.user.id,
}
# Validate URL if provided
if url_cover:
try:
validate_url_for_http_request(url_cover, "Cover URL")
except ValidationError as e:
log.error(f"Invalid cover URL in add_collection: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
db_collection = db_collection_handler.get_collection_by_name(
cleaned_data["name"], request.user.id
)
@@ -87,21 +79,25 @@ async def add_collection(
_added_collection = db_collection_handler.add_collection(Collection(**cleaned_data))
if artwork is not None and artwork.filename is not None:
file_ext = artwork.filename.split(".")[-1]
artwork_content = BytesIO(await artwork.read())
(
path_cover_l,
path_cover_s,
) = await fs_resource_handler.store_artwork(
_added_collection, artwork_content, file_ext
)
else:
path_cover_s, path_cover_l = await fs_resource_handler.get_cover(
entity=_added_collection,
overwrite=True,
url_cover=_added_collection.url_cover,
)
try:
if artwork is not None and artwork.filename is not None:
file_ext = artwork.filename.split(".")[-1]
artwork_content = BytesIO(await artwork.read())
(
path_cover_l,
path_cover_s,
) = await fs_resource_handler.store_artwork(
_added_collection, artwork_content, file_ext
)
else:
path_cover_s, path_cover_l = await fs_resource_handler.get_cover(
entity=_added_collection,
overwrite=True,
url_cover=_added_collection.url_cover,
)
except ValidationError as e:
log.error(f"Invalid cover URL in add_collection: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
_added_collection.path_cover_s = path_cover_s
_added_collection.path_cover_l = path_cover_l
@@ -448,26 +444,22 @@ async def update_collection(
current_url_cover != collection.url_cover
or not fs_resource_handler.cover_exists(collection, CoverSize.BIG)
):
# Validate URL if provided and changed
if current_url_cover:
try:
validate_url_for_http_request(current_url_cover, "Cover URL")
except ValidationError as e:
log.error(f"Invalid cover URL in update_collection: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
path_cover_s, path_cover_l = await fs_resource_handler.get_cover(
entity=collection,
overwrite=True,
url_cover=current_url_cover,
)
cleaned_data.update(
{
"url_cover": current_url_cover,
"path_cover_s": path_cover_s,
"path_cover_l": path_cover_l,
}
)
try:
path_cover_s, path_cover_l = await fs_resource_handler.get_cover(
entity=collection,
overwrite=True,
url_cover=current_url_cover,
)
cleaned_data.update(
{
"url_cover": current_url_cover,
"path_cover_s": path_cover_s,
"path_cover_l": path_cover_l,
}
)
except ValidationError as e:
log.error(f"Invalid cover URL in update_collection: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
updated_collection = db_collection_handler.update_collection(
id, cleaned_data, parsed_rom_ids

View File

@@ -73,7 +73,7 @@ from utils.filesystem import sanitize_filename
from utils.hashing import crc32_to_hex
from utils.nginx import FileRedirectResponse, ZipContentLine, ZipResponse
from utils.router import APIRouter
from utils.validation import ValidationError, validate_url_for_http_request
from utils.validation import ValidationError
router = APIRouter(
prefix="/roms",
@@ -1291,24 +1291,20 @@ async def update_rom(
cleaned_data.update({"igdb_id": None, "igdb_metadata": {}})
url_screenshots = cleaned_data.get("url_screenshots", [])
if url_screenshots:
for screenshot_url in url_screenshots:
try:
validate_url_for_http_request(str(screenshot_url), "Screenshot URL")
except ValidationError as e:
log.error(f"Invalid screenshot URL in update_rom: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
screenshots_changed = pydash.xor(url_screenshots, rom.url_screenshots or [])
if url_screenshots:
path_screenshots = await fs_resource_handler.get_rom_screenshots(
rom=rom,
overwrite=bool(screenshots_changed),
url_screenshots=cleaned_data.get("url_screenshots", []),
)
cleaned_data.update(
{"path_screenshots": path_screenshots, "url_screenshots": []}
)
try:
path_screenshots = await fs_resource_handler.get_rom_screenshots(
rom=rom,
overwrite=bool(screenshots_changed),
url_screenshots=cleaned_data.get("url_screenshots", []),
)
cleaned_data.update(
{"path_screenshots": path_screenshots, "url_screenshots": []}
)
except ValidationError as e:
log.error(f"Invalid screenshot URL in update_rom: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
cleaned_data.update(
{
@@ -1354,49 +1350,41 @@ async def update_rom(
url_cover = (
form_data.url_cover if "url_cover" in provided_fields else rom.url_cover
)
# Validate URL if provided and changed
if url_cover and url_cover != rom.url_cover:
try:
validate_url_for_http_request(url_cover, "Cover URL")
except ValidationError as e:
log.error(f"Invalid cover URL in update_rom: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
path_cover_s, path_cover_l = await fs_resource_handler.get_cover(
entity=rom,
overwrite=url_cover != rom.url_cover,
url_cover=str(url_cover),
)
cleaned_data.update(
{
"url_cover": url_cover,
"path_cover_s": path_cover_s,
"path_cover_l": path_cover_l,
}
)
try:
path_cover_s, path_cover_l = await fs_resource_handler.get_cover(
entity=rom,
overwrite=url_cover != rom.url_cover,
url_cover=str(url_cover),
)
cleaned_data.update(
{
"url_cover": url_cover,
"path_cover_s": path_cover_s,
"path_cover_l": path_cover_l,
}
)
except ValidationError as e:
log.error(f"Invalid cover URL in update_rom: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
url_manual = (
form_data.url_manual if "url_manual" in provided_fields else rom.url_manual
)
# Validate URL if provided and changed
if url_manual and url_manual != rom.url_manual:
try:
validate_url_for_http_request(url_manual, "Manual URL")
except ValidationError as e:
log.error(f"Invalid manual URL in update_rom: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
path_manual = await fs_resource_handler.get_manual(
rom=rom,
overwrite=url_manual != rom.url_manual,
url_manual=str(url_manual) if url_manual else None,
)
cleaned_data.update(
{
"url_manual": url_manual,
"path_manual": path_manual,
}
)
try:
path_manual = await fs_resource_handler.get_manual(
rom=rom,
overwrite=url_manual != rom.url_manual,
url_manual=str(url_manual) if url_manual else None,
)
cleaned_data.update(
{
"url_manual": url_manual,
"path_manual": path_manual,
}
)
except ValidationError as e:
log.error(f"Invalid manual URL in update_rom: {str(e)}")
raise HTTPException(status_code=400, detail=str(e)) from e
# Handle RetroAchievements badges when the ID has changed
if cleaned_data["ra_id"] and int(cleaned_data["ra_id"]) != rom.ra_id:

View File

@@ -95,11 +95,7 @@ class FSResourcesHandler(FSHandler):
else:
# Handle HTTP URLs
# Validate URL to prevent SSRF attacks
try:
validate_url_for_http_request(url_cover, "Cover URL")
except ValidationError as e:
log.error(f"URL validation failed for cover: {str(e)}")
return None
validate_url_for_http_request(url_cover, "Cover URL")
httpx_client = ctx_httpx_client.get()
try:
@@ -268,11 +264,7 @@ class FSResourcesHandler(FSHandler):
else:
# Handle HTTP URLs
# Validate URL to prevent SSRF attacks
try:
validate_url_for_http_request(url_screenhot, "Screenshot URL")
except ValidationError as e:
log.error(f"URL validation failed for screenshot: {str(e)}")
return None
validate_url_for_http_request(url_screenhot, "Screenshot URL")
httpx_client = ctx_httpx_client.get()
try:
@@ -388,11 +380,7 @@ class FSResourcesHandler(FSHandler):
else:
# Handle HTTP URL
# Validate URL to prevent SSRF attacks
try:
validate_url_for_http_request(url_manual, "Manual URL")
except ValidationError as e:
log.error(f"URL validation failed for manual: {str(e)}")
return None
validate_url_for_http_request(url_manual, "Manual URL")
httpx_client = ctx_httpx_client.get()
try:
@@ -453,11 +441,7 @@ class FSResourcesHandler(FSHandler):
# Retroachievements
async def store_ra_badge(self, url: str, path: str) -> None:
# Validate URL to prevent SSRF attacks
try:
validate_url_for_http_request(url, "Badge URL")
except ValidationError as e:
log.error(f"URL validation failed for badge: {str(e)}")
return
validate_url_for_http_request(url, "Badge URL")
httpx_client = ctx_httpx_client.get()
directory, filename = os.path.split(path)
@@ -523,11 +507,7 @@ class FSResourcesHandler(FSHandler):
else:
# Handle HTTP URLs
# Validate URL to prevent SSRF attacks
try:
validate_url_for_http_request(url, "Media URL")
except ValidationError as e:
log.error(f"URL validation failed for media file: {str(e)}")
return None
validate_url_for_http_request(url, "Media URL")
httpx_client = ctx_httpx_client.get()
try:

View File

@@ -47,7 +47,7 @@ export function useFavoriteToggle(emitter?: Emitter<Events>) {
const currentlyFav = fav.rom_ids.includes(rom.id);
if (currentlyFav) {
fav.rom_ids = fav.rom_ids.filter((id: number) => id !== rom.id);
fav.rom_ids = fav.rom_ids.filter((id) => id !== rom.id);
if (romsStore.currentCollection?.id === fav.id) {
romsStore.remove([rom]);
}
@@ -72,7 +72,7 @@ export function useFavoriteToggle(emitter?: Emitter<Events>) {
if (currentlyFav) {
fav.rom_ids.push(rom.id);
} else {
fav.rom_ids = fav.rom_ids.filter((id: number) => id !== rom.id);
fav.rom_ids = fav.rom_ids.filter((id) => id !== rom.id);
}
const detail = (error as { response?: { data?: { detail?: string } } })
?.response?.data?.detail;

View File

@@ -11,7 +11,6 @@ import {
} from "vue";
import { useI18n } from "vue-i18n";
import { useRoute, useRouter } from "vue-router";
import type { DetailedRomSchema } from "@/__generated__/models/DetailedRomSchema";
import BackButton from "@/console/components/BackButton.vue";
import NavigationHint from "@/console/components/NavigationHint.vue";
import NavigationText from "@/console/components/NavigationText.vue";
@@ -22,6 +21,7 @@ import { ROUTES } from "@/plugins/router";
import romApi from "@/services/api/rom";
import stateApi from "@/services/api/state";
import storeHeartbeat from "@/stores/heartbeat";
import type { DetailedRom } from "@/stores/roms";
import storeRoms from "@/stores/roms";
import { getSupportedEJSCores, toBrowserLocale } from "@/utils";
import {
@@ -46,7 +46,7 @@ const route = useRoute();
const router = useRouter();
const { t, locale } = useI18n();
const rom = ref<DetailedRomSchema | null>(null);
const rom = ref<DetailedRom | null>(null);
const playerState = ref<PlayerState>("loading");
const errorMessage = ref<string | null>(null);

View File

@@ -15,7 +15,6 @@ import type {
Body_add_state_api_states_post as AddStateInput,
FirmwareSchema,
} from "@/__generated__";
import type { DetailedRomSchema } from "@/__generated__/models/DetailedRomSchema";
import NavigationText from "@/console/components/NavigationText.vue";
import { useInputScope } from "@/console/composables/useInputScope";
import { useThemeAssets } from "@/console/composables/useThemeAssets";
@@ -25,6 +24,7 @@ import firmwareApi from "@/services/api/firmware";
import romApi from "@/services/api/rom";
import storeConfig from "@/stores/config";
import storeLanguage from "@/stores/language";
import type { DetailedRom } from "@/stores/roms";
import {
getSupportedEJSCores,
getControlSchemeForPlatform,
@@ -60,7 +60,7 @@ const { selectedLanguage } = storeToRefs(languageStore);
const romId = Number(route.params.rom);
const initialSaveId = route.query.save ? Number(route.query.save) : null;
const initialStateId = route.query.state ? Number(route.query.state) : null;
const romRef = ref<DetailedRomSchema | null>(null);
const romRef = ref<DetailedRom | null>(null);
const showHint = ref(true);
const bezelSrc = ref<string>("");
const showExitPrompt = ref(false);

View File

@@ -1,5 +1,5 @@
import type { PlatformSchema } from "@/__generated__";
import api from "@/services/api";
import type { Platform } from "@/stores/platforms";
export type LibraryStructure = "struct_a" | "struct_b" | null;
@@ -11,7 +11,7 @@ export interface ExistingPlatform {
export interface SetupLibraryInfo {
detected_structure: LibraryStructure;
existing_platforms: ExistingPlatform[];
supported_platforms: PlatformSchema[];
supported_platforms: Platform[];
}
export interface CreatePlatformsResponse {

View File

@@ -62,13 +62,6 @@ async function updateUser({
avatar?: File;
password?: string;
}) {
const uiSettings =
typeof attrs.ui_settings === "string"
? attrs.ui_settings
: attrs.ui_settings
? JSON.stringify(attrs.ui_settings)
: attrs.ui_settings;
return api.put<UserSchema>(
`/users/${id}`,
{
@@ -79,7 +72,7 @@ async function updateUser({
enabled: attrs.enabled,
role: attrs.role,
ra_username: attrs.ra_username,
ui_settings: uiSettings,
ui_settings: attrs.ui_settings,
},
{
headers: {

View File

@@ -1,8 +1,6 @@
import { defineStore } from "pinia";
import type { PlatformSchema } from "@/__generated__";
import { romStatusMap } from "@/utils";
export type Platform = PlatformSchema;
import type { Platform } from "./platforms";
export type FilterType =
| "genres"