From c2c9f7a21f6d91f4dd043fc4e80dd4ac52a99e57 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Thu, 2 May 2024 13:39:08 +0100 Subject: [PATCH 01/11] Soft validate image block size, compression and bootable markers --- app/forms/image-upload.tsx | 215 +++++++++++++++++++++++++++++++++---- app/util/links.ts | 2 + 2 files changed, 197 insertions(+), 20 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 087e852ec3..2a2bade454 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -9,7 +9,7 @@ import cn from 'classnames' import { filesize } from 'filesize' import pMap from 'p-map' import pRetry from 'p-retry' -import { useRef, useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { useNavigate } from 'react-router-dom' import { @@ -22,6 +22,7 @@ import { } from '@oxide/api' import { Error12Icon, + OpenLink12Icon, Success12Icon, Unauthorized12Icon, } from '@oxide/design-system/icons/react' @@ -40,6 +41,7 @@ import { Spinner } from '~/ui/lib/Spinner' import { anySignal } from '~/util/abort' import { readBlobAsBase64 } from '~/util/file' import { invariant } from '~/util/invariant' +import { links } from '~/util/links' import { pb } from '~/util/path-builder' import { GiB, KiB } from '~/util/units' @@ -477,6 +479,79 @@ export function CreateImageSideModalForm() { const form = useForm({ defaultValues }) const file = form.watch('imageFile') + const blockSize = form.watch('blockSize') + + const { efiPart, isBootableCd, isCompressed } = useValidateImage(file) + + const BlockSizeNotice = () => { + if (!file || efiPart === -1) return null + + if (blockSize !== efiPart) { + return ( + + ) + } + } + + const BootableNotice = () => { + if (!file) return null + + // this message should only appear if the image doesn't have a header + // marker we are looking for and does not appear to be compressed + if (((efiPart !== -1 && efiPart !== null) || isBootableCd) && !isCompressed) { + return null + } + + const content = ( +
+
    + {efiPart === -1 && !isBootableCd && ( +
  • +
    Bootable markers not found at any block size
    +
    + Expected either “EFI PART” marker at offsets 512 / 2048 / 4096 or “CD001” at + offset 0x8001 (for a bootable CD). +
    +
    • + )} + {isCompressed && ( +
  • +
    This might be a compressed image
    +
    + Only raw, uncompressed images are supported. Files such as qcow2, vmdk, + img.gz, iso.7z may not work. +
    +
    • + )} +
+
+ Learn more about{' '} + + preparing images for import + + +
+
+ ) + + return ( + + ) + } return ( - parseInt(val, 10) as BlockSize} - items={[ - { label: '512', value: 512 }, - { label: '2048', value: 2048 }, - { label: '4096', value: 4096 }, - ]} - /> - +
+ parseInt(val, 10) as BlockSize} + items={[ + { label: '512', value: 512 }, + { label: '2048', value: 2048 }, + { label: '4096', value: 4096 }, + ]} + /> + +
+
+ + +
{file && modalOpen && ( @@ -640,3 +721,97 @@ export function CreateImageSideModalForm() { ) } + +const useValidateImage = ( + file: File | null +): { + efiPart: number | null + isBootableCd: boolean | null + isCompressed: boolean | null +} => { + const [efiPart, setEfiPart] = useState(null) + const [isBootableCd, setIsBootableCd] = useState(null) + const [isCompressed, setIsCompressed] = useState(null) + + useEffect(() => { + setEfiPart(null) + setIsBootableCd(null) + setIsCompressed(null) + + const readAndMatch = async ( + offset: number, + length: number, + matchString: string + ): Promise => { + if (!file) return false + + const slice = file.slice(offset, offset + length) + const reader = new FileReader() + + const promise = new Promise((resolve, reject) => { + reader.onloadend = (e) => { + if (!e || !e.target) { + resolve(false) + return + } + + if (e.target.readyState === FileReader.DONE) { + if (e.target.result instanceof ArrayBuffer) { + const actualHeader = String.fromCharCode(...new Uint8Array(e.target.result)) + resolve(actualHeader === matchString) + } else { + resolve(false) + } + } + } + + reader.onerror = (error) => { + console.error(`Error reading file at offset ${offset}:`, error) + reject(error) + } + }) + + reader.readAsArrayBuffer(slice) + return await promise + } + + const checkEfiPart = async (): Promise => { + const offsets = [512, 2048, 4096] + for (const offset of offsets) { + const isMatch = await readAndMatch(offset, 8, 'EFI PART') + if (isMatch) return offset + } + return -1 + } + + const checkBootableCd = async (): Promise => { + return await readAndMatch(0x8001, 5, 'CD001') + } + + const checkCompression = (fileName: string): boolean => { + const lowerFileName = fileName.toLowerCase() + return ( + lowerFileName.endsWith('.qcow2') || + lowerFileName.endsWith('.vmdk') || + lowerFileName.endsWith('.gz') || + lowerFileName.endsWith('.7z') + ) + } + + const performChecks = async () => { + if (file) { + const efiPartResult = await checkEfiPart() + const bootableCdResult = await checkBootableCd() + const compressedResult = checkCompression(file.name) + + setEfiPart(efiPartResult) + setIsBootableCd(bootableCdResult) + setIsCompressed(compressedResult) + } + } + + performChecks() + }, [file]) + + return { efiPart, isBootableCd, isCompressed } +} diff --git a/app/util/links.ts b/app/util/links.ts index 9fa5cabbae..8ed991d199 100644 --- a/app/util/links.ts +++ b/app/util/links.ts @@ -14,6 +14,8 @@ export const links: Record = { 'https://docs.oxide.computer/guides/configuring-guest-networking#_firewall_rules', floatingIpsDocs: 'https://docs.oxide.computer/guides/managing-floating-ips', imagesDocs: 'https://docs.oxide.computer/guides/creating-and-sharing-images', + preparingImagesDocs: + 'https://docs.oxide.computer/guides/creating-and-sharing-images#_preparing_images_for_import', instancesDocs: 'https://docs.oxide.computer/guides/managing-instances', keyConceptsIamPolicyDocs: 'https://docs.oxide.computer/guides/key-entities-and-concepts#iam-policy', From 06aa114827008ffc9758408f29752389bbca07dd Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Thu, 2 May 2024 14:46:07 +0100 Subject: [PATCH 02/11] Handle bootable CD block size --- app/forms/image-upload.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 2a2bade454..ab9cbc6008 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -484,14 +484,20 @@ export function CreateImageSideModalForm() { const { efiPart, isBootableCd, isCompressed } = useValidateImage(file) const BlockSizeNotice = () => { - if (!file || efiPart === -1) return null + if (!file || (efiPart === -1 && !isBootableCd)) return null + + let content = `Detected "EFI PART" marker at offset ${efiPart}, but block size is set to ${blockSize}.` + + if (isBootableCd && blockSize === 4096) { + content = 'Bootable CDs typically use a block size of 2048.' + } if (blockSize !== efiPart) { return ( ) } From 81f4c3c15c7f7819afd26342bf54bbf22be8c976 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 May 2024 13:48:23 -0500 Subject: [PATCH 03/11] move file notice component definitions out of render --- app/forms/image-upload.tsx | 186 +++++++++++++++++++++---------------- 1 file changed, 108 insertions(+), 78 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index ab9cbc6008..7cc4fc2cab 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -483,82 +483,6 @@ export function CreateImageSideModalForm() { const { efiPart, isBootableCd, isCompressed } = useValidateImage(file) - const BlockSizeNotice = () => { - if (!file || (efiPart === -1 && !isBootableCd)) return null - - let content = `Detected "EFI PART" marker at offset ${efiPart}, but block size is set to ${blockSize}.` - - if (isBootableCd && blockSize === 4096) { - content = 'Bootable CDs typically use a block size of 2048.' - } - - if (blockSize !== efiPart) { - return ( - - ) - } - } - - const BootableNotice = () => { - if (!file) return null - - // this message should only appear if the image doesn't have a header - // marker we are looking for and does not appear to be compressed - if (((efiPart !== -1 && efiPart !== null) || isBootableCd) && !isCompressed) { - return null - } - - const content = ( -
-
    - {efiPart === -1 && !isBootableCd && ( -
  • -
    Bootable markers not found at any block size
    -
    - Expected either “EFI PART” marker at offsets 512 / 2048 / 4096 or “CD001” at - offset 0x8001 (for a bootable CD). -
    -
    • - )} - {isCompressed && ( -
  • -
    This might be a compressed image
    -
    - Only raw, uncompressed images are supported. Files such as qcow2, vmdk, - img.gz, iso.7z may not work. -
    -
    • - )} -
-
- Learn more about{' '} - - preparing images for import - - -
-
- ) - - return ( - - ) - } - return ( - +
- +
{file && modalOpen && ( @@ -728,6 +662,102 @@ export function CreateImageSideModalForm() { ) } +function BlockSizeNotice({ + file, + blockSize, + efiPart, + isBootableCd, +}: { + file: File | null + blockSize: number | null + efiPart: number | null + isBootableCd: boolean | null +}) { + if (!file || (efiPart === -1 && !isBootableCd)) return null + + let content = `Detected "EFI PART" marker at offset ${efiPart}, but block size is set to ${blockSize}.` + + if (isBootableCd && blockSize === 4096) { + content = 'Bootable CDs typically use a block size of 2048.' + } + + if (blockSize !== efiPart) { + return ( + + ) + } +} + +function BootableNotice({ + file, + efiPart, + isBootableCd, + isCompressed, +}: { + file: File | null + efiPart: number | null + isBootableCd: boolean | null + isCompressed: boolean | null +}) { + if (!file) return null + + // this message should only appear if the image doesn't have a header + // marker we are looking for and does not appear to be compressed + if (((efiPart !== -1 && efiPart !== null) || isBootableCd) && !isCompressed) { + return null + } + + const content = ( +
+
    + {efiPart === -1 && !isBootableCd && ( +
  • +
    Bootable markers not found at any block size
    +
    + Expected either “EFI PART” marker at offsets 512 / 2048 / 4096 or “CD001” at + offset 0x8001 (for a bootable CD). +
    +
    • + )} + {isCompressed && ( +
  • +
    This might be a compressed image
    +
    + Only raw, uncompressed images are supported. Files such as qcow2, vmdk, + img.gz, iso.7z may not work. +
    +
    • + )} +
+
+ Learn more about{' '} + + preparing images for import + + +
+
+ ) + + return ( + + ) +} + const useValidateImage = ( file: File | null ): { From 3fba79e705facdd8bc9d736757fae750e4dd3dca Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 May 2024 13:55:11 -0500 Subject: [PATCH 04/11] extract inner functions from useValidateImage --- app/forms/image-upload.tsx | 120 ++++++++++++++++++------------------- 1 file changed, 58 insertions(+), 62 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 7cc4fc2cab..35a27d2b2b 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -757,6 +757,62 @@ function BootableNotice({ /> ) } +async function readAndMatch( + file: File | null, + offset: number, + length: number, + matchString: string +): Promise { + if (!file) return false + + const slice = file.slice(offset, offset + length) + const reader = new FileReader() + + const promise = new Promise((resolve, reject) => { + reader.onloadend = (e) => { + if (!e || !e.target) { + resolve(false) + return + } + + if (e.target.readyState === FileReader.DONE) { + if (e.target.result instanceof ArrayBuffer) { + const actualHeader = String.fromCharCode(...new Uint8Array(e.target.result)) + resolve(actualHeader === matchString) + } else { + resolve(false) + } + } + } + + reader.onerror = (error) => { + console.error(`Error reading file at offset ${offset}:`, error) + reject(error) + } + }) + + reader.readAsArrayBuffer(slice) + return await promise +} + +async function checkEfiPart(file: File | null): Promise { + const offsets = [512, 2048, 4096] + for (const offset of offsets) { + const isMatch = await readAndMatch(file, offset, 8, 'EFI PART') + if (isMatch) return offset + } + return -1 +} + +function checkCompression(fileName: string) { + const lowerFileName = fileName.toLowerCase() + return ( + lowerFileName.endsWith('.qcow2') || + lowerFileName.endsWith('.vmdk') || + lowerFileName.endsWith('.gz') || + lowerFileName.endsWith('.7z') + ) +} const useValidateImage = ( file: File | null @@ -774,70 +830,10 @@ const useValidateImage = ( setIsBootableCd(null) setIsCompressed(null) - const readAndMatch = async ( - offset: number, - length: number, - matchString: string - ): Promise => { - if (!file) return false - - const slice = file.slice(offset, offset + length) - const reader = new FileReader() - - const promise = new Promise((resolve, reject) => { - reader.onloadend = (e) => { - if (!e || !e.target) { - resolve(false) - return - } - - if (e.target.readyState === FileReader.DONE) { - if (e.target.result instanceof ArrayBuffer) { - const actualHeader = String.fromCharCode(...new Uint8Array(e.target.result)) - resolve(actualHeader === matchString) - } else { - resolve(false) - } - } - } - - reader.onerror = (error) => { - console.error(`Error reading file at offset ${offset}:`, error) - reject(error) - } - }) - - reader.readAsArrayBuffer(slice) - return await promise - } - - const checkEfiPart = async (): Promise => { - const offsets = [512, 2048, 4096] - for (const offset of offsets) { - const isMatch = await readAndMatch(offset, 8, 'EFI PART') - if (isMatch) return offset - } - return -1 - } - - const checkBootableCd = async (): Promise => { - return await readAndMatch(0x8001, 5, 'CD001') - } - - const checkCompression = (fileName: string): boolean => { - const lowerFileName = fileName.toLowerCase() - return ( - lowerFileName.endsWith('.qcow2') || - lowerFileName.endsWith('.vmdk') || - lowerFileName.endsWith('.gz') || - lowerFileName.endsWith('.7z') - ) - } - const performChecks = async () => { if (file) { - const efiPartResult = await checkEfiPart() - const bootableCdResult = await checkBootableCd() + const efiPartResult = await checkEfiPart(file) + const bootableCdResult = await readAndMatch(file, 0x8001, 5, 'CD001') const compressedResult = checkCompression(file.name) setEfiPart(efiPartResult) From 458d07b1b7f00f47deafe3fd5922a20c5f06c915 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 May 2024 14:03:42 -0500 Subject: [PATCH 05/11] rename function --- app/forms/image-upload.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 35a27d2b2b..6ef5278ccb 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -757,6 +757,7 @@ function BootableNotice({ /> ) } + async function readAndMatch( file: File | null, offset: number, @@ -795,6 +796,7 @@ async function readAndMatch( return await promise } +// TODO: use Promise.all for these async function checkEfiPart(file: File | null): Promise { const offsets = [512, 2048, 4096] for (const offset of offsets) { @@ -804,14 +806,10 @@ async function checkEfiPart(file: File | null): Promise { return -1 } -function checkCompression(fileName: string) { +const compressedExts = ['.gz', '.7z', '.qcow2', '.vmdk'] +function usesCompressedExt(fileName: string) { const lowerFileName = fileName.toLowerCase() - return ( - lowerFileName.endsWith('.qcow2') || - lowerFileName.endsWith('.vmdk') || - lowerFileName.endsWith('.gz') || - lowerFileName.endsWith('.7z') - ) + return compressedExts.some((ext) => lowerFileName.endsWith(ext)) } const useValidateImage = ( @@ -834,7 +832,7 @@ const useValidateImage = ( if (file) { const efiPartResult = await checkEfiPart(file) const bootableCdResult = await readAndMatch(file, 0x8001, 5, 'CD001') - const compressedResult = checkCompression(file.name) + const compressedResult = usesCompressedExt(file.name) setEfiPart(efiPartResult) setIsBootableCd(bootableCdResult) From 63d72e50b00bbebaea5f3fd1fc8d3657764dbc1f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 May 2024 14:26:00 -0500 Subject: [PATCH 06/11] use useQuery to do the image validation --- app/forms/image-upload.tsx | 62 +++++++++----------------------------- app/main.tsx | 4 +-- 2 files changed, 17 insertions(+), 49 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 6ef5278ccb..df1e42a585 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -5,11 +5,12 @@ * * Copyright Oxide Computer Company */ +import { skipToken, useQuery } from '@tanstack/react-query' import cn from 'classnames' import { filesize } from 'filesize' import pMap from 'p-map' import pRetry from 'p-retry' -import { useEffect, useRef, useState } from 'react' +import { useRef, useState } from 'react' import { useNavigate } from 'react-router-dom' import { @@ -481,7 +482,10 @@ export function CreateImageSideModalForm() { const file = form.watch('imageFile') const blockSize = form.watch('blockSize') - const { efiPart, isBootableCd, isCompressed } = useValidateImage(file) + const { data: imageValidation } = useQuery({ + queryKey: ['validateImage', ...(file ? [file.name, file.size, file.lastModified] : [])], + queryFn: file ? () => performChecks(file) : skipToken, + }) return ( - + {imageValidation && ( + + )}
- + {imageValidation && }
{file && modalOpen && ( @@ -812,36 +808,8 @@ function usesCompressedExt(fileName: string) { return compressedExts.some((ext) => lowerFileName.endsWith(ext)) } -const useValidateImage = ( - file: File | null -): { - efiPart: number | null - isBootableCd: boolean | null - isCompressed: boolean | null -} => { - const [efiPart, setEfiPart] = useState(null) - const [isBootableCd, setIsBootableCd] = useState(null) - const [isCompressed, setIsCompressed] = useState(null) - - useEffect(() => { - setEfiPart(null) - setIsBootableCd(null) - setIsCompressed(null) - - const performChecks = async () => { - if (file) { - const efiPartResult = await checkEfiPart(file) - const bootableCdResult = await readAndMatch(file, 0x8001, 5, 'CD001') - const compressedResult = usesCompressedExt(file.name) - - setEfiPart(efiPartResult) - setIsBootableCd(bootableCdResult) - setIsCompressed(compressedResult) - } - } - - performChecks() - }, [file]) - - return { efiPart, isBootableCd, isCompressed } -} +const performChecks = async (file: File) => ({ + efiPart: await checkEfiPart(file), + isBootableCd: await readAndMatch(file, 0x8001, 5, 'CD001'), + isCompressed: usesCompressedExt(file.name), +}) diff --git a/app/main.tsx b/app/main.tsx index 20cde9eb73..946b61c37b 100644 --- a/app/main.tsx +++ b/app/main.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ import { QueryClientProvider } from '@tanstack/react-query' -// import { ReactQueryDevtools } from '@tanstack/react-query-devtools' +import { ReactQueryDevtools } from '@tanstack/react-query-devtools' import { StrictMode } from 'react' import { createRoot } from 'react-dom/client' import { createBrowserRouter, RouterProvider } from 'react-router-dom' @@ -54,7 +54,7 @@ function render() { - {/* */} + ) From b9e7e78dc8730a95bdbb40621fe67ce1df90c837 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 May 2024 14:30:05 -0500 Subject: [PATCH 07/11] non-nullable props, rename efiOffset --- app/forms/image-upload.tsx | 48 ++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index df1e42a585..cb5e294772 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -484,7 +484,7 @@ export function CreateImageSideModalForm() { const { data: imageValidation } = useQuery({ queryKey: ['validateImage', ...(file ? [file.name, file.size, file.lastModified] : [])], - queryFn: file ? () => performChecks(file) : skipToken, + queryFn: file ? () => validateImage(file) : skipToken, }) return ( @@ -661,23 +661,23 @@ export function CreateImageSideModalForm() { function BlockSizeNotice({ file, blockSize, - efiPart, + efiPartOffset, isBootableCd, }: { file: File | null - blockSize: number | null - efiPart: number | null - isBootableCd: boolean | null + blockSize: number + efiPartOffset: number + isBootableCd: boolean }) { - if (!file || (efiPart === -1 && !isBootableCd)) return null + if (!file || (efiPartOffset === -1 && !isBootableCd)) return null - let content = `Detected "EFI PART" marker at offset ${efiPart}, but block size is set to ${blockSize}.` + let content = `Detected "EFI PART" marker at offset ${efiPartOffset}, but block size is set to ${blockSize}.` if (isBootableCd && blockSize === 4096) { content = 'Bootable CDs typically use a block size of 2048.' } - if (blockSize !== efiPart) { + if (blockSize !== efiPartOffset) { return (
    - {efiPart === -1 && !isBootableCd && ( + {efiPartOffset === -1 && !isBootableCd && (
  • Bootable markers not found at any block size
    @@ -792,8 +792,7 @@ async function readAndMatch( return await promise } -// TODO: use Promise.all for these -async function checkEfiPart(file: File | null): Promise { +async function checkEfiPart(file: File | null) { const offsets = [512, 2048, 4096] for (const offset of offsets) { const isMatch = await readAndMatch(file, offset, 8, 'EFI PART') @@ -803,13 +802,12 @@ async function checkEfiPart(file: File | null): Promise { } const compressedExts = ['.gz', '.7z', '.qcow2', '.vmdk'] -function usesCompressedExt(fileName: string) { - const lowerFileName = fileName.toLowerCase() - return compressedExts.some((ext) => lowerFileName.endsWith(ext)) -} -const performChecks = async (file: File) => ({ - efiPart: await checkEfiPart(file), - isBootableCd: await readAndMatch(file, 0x8001, 5, 'CD001'), - isCompressed: usesCompressedExt(file.name), -}) +const validateImage = async (file: File) => { + const lowerFileName = file.name.toLowerCase() + return { + efiPartOffset: await checkEfiPart(file), + isBootableCd: await readAndMatch(file, 0x8001, 5, 'CD001'), + isCompressed: compressedExts.some((ext) => lowerFileName.endsWith(ext)), + } +} From aae094e92432ab2c6e591db8f70d50e2d2baee49 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 2 May 2024 14:48:56 -0500 Subject: [PATCH 08/11] clean up stuff more --- app/forms/image-upload.tsx | 52 ++++++++++++++------------------------ app/main.tsx | 4 +-- 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index cb5e294772..8e19a0ca02 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -567,9 +567,7 @@ export function CreateImageSideModalForm() { { label: '4096', value: 4096 }, ]} /> - {imageValidation && ( - - )} + {imageValidation && }
    - {imageValidation && } + {imageValidation && }
    {file && modalOpen && ( @@ -659,58 +657,48 @@ export function CreateImageSideModalForm() { } function BlockSizeNotice({ - file, blockSize, efiPartOffset, isBootableCd, }: { - file: File | null blockSize: number efiPartOffset: number isBootableCd: boolean }) { - if (!file || (efiPartOffset === -1 && !isBootableCd)) return null + if (efiPartOffset === -1 && !isBootableCd) return null - let content = `Detected "EFI PART" marker at offset ${efiPartOffset}, but block size is set to ${blockSize}.` + // TODO: make sure this logic is correct - if (isBootableCd && blockSize === 4096) { - content = 'Bootable CDs typically use a block size of 2048.' - } + if (blockSize === efiPartOffset) return null - if (blockSize !== efiPartOffset) { - return ( - - ) - } + const content = + isBootableCd && blockSize !== 2048 + ? 'Bootable CDs typically use a block size of 2048.' + : `Detected "EFI PART" marker at offset ${efiPartOffset}, but block size is set to ${blockSize}.` + + return ( + + ) } function BootableNotice({ - file, efiPartOffset, isBootableCd, isCompressed, }: { - file: File | null efiPartOffset: number isBootableCd: boolean isCompressed: boolean }) { - if (!file) return null - // this message should only appear if the image doesn't have a header // marker we are looking for and does not appear to be compressed - if ((efiPartOffset !== -1 || isBootableCd) && !isCompressed) { - return null - } + const efiPartOrBootable = efiPartOffset !== -1 || isBootableCd + if (efiPartOrBootable && !isCompressed) return null const content = (
      - {efiPartOffset === -1 && !isBootableCd && ( + {!efiPartOrBootable && (
    • Bootable markers not found at any block size
      @@ -755,13 +743,11 @@ function BootableNotice({ } async function readAndMatch( - file: File | null, + file: File, offset: number, length: number, matchString: string ): Promise { - if (!file) return false - const slice = file.slice(offset, offset + length) const reader = new FileReader() @@ -792,7 +778,7 @@ async function readAndMatch( return await promise } -async function checkEfiPart(file: File | null) { +async function getEfiPartOffset(file: File) { const offsets = [512, 2048, 4096] for (const offset of offsets) { const isMatch = await readAndMatch(file, offset, 8, 'EFI PART') @@ -806,7 +792,7 @@ const compressedExts = ['.gz', '.7z', '.qcow2', '.vmdk'] const validateImage = async (file: File) => { const lowerFileName = file.name.toLowerCase() return { - efiPartOffset: await checkEfiPart(file), + efiPartOffset: await getEfiPartOffset(file), isBootableCd: await readAndMatch(file, 0x8001, 5, 'CD001'), isCompressed: compressedExts.some((ext) => lowerFileName.endsWith(ext)), } diff --git a/app/main.tsx b/app/main.tsx index 946b61c37b..20cde9eb73 100644 --- a/app/main.tsx +++ b/app/main.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ import { QueryClientProvider } from '@tanstack/react-query' -import { ReactQueryDevtools } from '@tanstack/react-query-devtools' +// import { ReactQueryDevtools } from '@tanstack/react-query-devtools' import { StrictMode } from 'react' import { createRoot } from 'react-dom/client' import { createBrowserRouter, RouterProvider } from 'react-router-dom' @@ -54,7 +54,7 @@ function render() { - + {/* */} ) From ee8c923b7d1e6330a63e896268709d5db4d923df Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Fri, 17 May 2024 15:05:38 +0100 Subject: [PATCH 09/11] Remove tertiary styling --- app/forms/image-upload.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 8e19a0ca02..26d32d1660 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -700,8 +700,8 @@ function BootableNotice({
        {!efiPartOrBootable && (
      • -
        Bootable markers not found at any block size
        -
        +
        Bootable markers not found at any block size.
        +
        Expected either “EFI PART” marker at offsets 512 / 2048 / 4096 or “CD001” at offset 0x8001 (for a bootable CD).
        @@ -709,8 +709,8 @@ function BootableNotice({ )} {isCompressed && (
      • -
        This might be a compressed image
        -
        +
        This might be a compressed image.
        +
        Only raw, uncompressed images are supported. Files such as qcow2, vmdk, img.gz, iso.7z may not work.
        From dbc51c2ac16ea828bf2cef8582b3e03958bd2b05 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 17 May 2024 10:06:40 -0500 Subject: [PATCH 10/11] tweak read file at offset logic --- app/forms/image-upload.tsx | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 26d32d1660..a5290e0879 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -742,30 +742,20 @@ function BootableNotice({ ) } -async function readAndMatch( - file: File, - offset: number, - length: number, - matchString: string -): Promise { - const slice = file.slice(offset, offset + length) +async function readAtOffset(file: File, offset: number, length: number) { const reader = new FileReader() - const promise = new Promise((resolve, reject) => { + const promise = new Promise((resolve, reject) => { reader.onloadend = (e) => { - if (!e || !e.target) { - resolve(false) + if ( + e.target?.readyState === FileReader.DONE && + // should always be true because we're using readAsArrayBuffer + e.target.result instanceof ArrayBuffer + ) { + resolve(String.fromCharCode(...new Uint8Array(e.target.result))) return } - - if (e.target.readyState === FileReader.DONE) { - if (e.target.result instanceof ArrayBuffer) { - const actualHeader = String.fromCharCode(...new Uint8Array(e.target.result)) - resolve(actualHeader === matchString) - } else { - resolve(false) - } - } + resolve(undefined) } reader.onerror = (error) => { @@ -774,14 +764,14 @@ async function readAndMatch( } }) - reader.readAsArrayBuffer(slice) - return await promise + reader.readAsArrayBuffer(file.slice(offset, offset + length)) + return promise } async function getEfiPartOffset(file: File) { const offsets = [512, 2048, 4096] for (const offset of offsets) { - const isMatch = await readAndMatch(file, offset, 8, 'EFI PART') + const isMatch = (await readAtOffset(file, offset, 8)) === 'EFI PART' if (isMatch) return offset } return -1 @@ -793,7 +783,7 @@ const validateImage = async (file: File) => { const lowerFileName = file.name.toLowerCase() return { efiPartOffset: await getEfiPartOffset(file), - isBootableCd: await readAndMatch(file, 0x8001, 5, 'CD001'), + isBootableCd: (await readAtOffset(file, 0x8001, 5)) === 'CD001', isCompressed: compressedExts.some((ext) => lowerFileName.endsWith(ext)), } } From f8f21d1cd96696c5cb601a5ad47d6d640d2f5292 Mon Sep 17 00:00:00 2001 From: iliana etaoin Date: Mon, 20 May 2024 09:17:33 -0700 Subject: [PATCH 11/11] correct logic around bootable CDs --- app/forms/image-upload.tsx | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index a5290e0879..b3be957b9c 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -665,16 +665,20 @@ function BlockSizeNotice({ efiPartOffset: number isBootableCd: boolean }) { - if (efiPartOffset === -1 && !isBootableCd) return null - - // TODO: make sure this logic is correct - - if (blockSize === efiPartOffset) return null - - const content = - isBootableCd && blockSize !== 2048 - ? 'Bootable CDs typically use a block size of 2048.' - : `Detected "EFI PART" marker at offset ${efiPartOffset}, but block size is set to ${blockSize}.` + const isEfi = efiPartOffset !== -1 + + // If the image doesn't look bootable, return null (`BootableNotice` does the work). + if (!isEfi && !isBootableCd) return null + // If we detect `EFI BOOT` and the block size is set correctly return null. + // (This includes hybrid GPT+ISO.) + if (isEfi && blockSize === efiPartOffset) return null + // If we detect only `CD001` and the block size is set correctly return null. + if (!isEfi && isBootableCd && blockSize === 2048) return null + + // Block size is set incorrectly. If we detect `EFI BOOT`, always show that warning. + const content = isEfi + ? `Detected “EFI PART” marker at offset ${efiPartOffset}, but block size is set to ${blockSize}.` + : 'Bootable CDs typically use a block size of 2048.' return (