fix(boot): fix bugs found in a review of the bootloader

Cargo.toml

@@ -50,10 +50,6 @@ default-features = false
 version = "0.11.0"
 default-features = false
 
-[workspace.dependencies.shlex]
-version = "2.0.1"
-default-features = false
-
 [workspace.dependencies.toml]
 version = "1.1.6"
 default-features = false

crates/boot/src/context.rs

@@ -233,7 +233,19 @@ impl SproutContext {
     /// For example, if this context contains {"a":"b"}, and the text "hello\\$a", it will produce
     /// "hello\\b" as an output string.
     pub fn stamp(&self, text: impl AsRef<str>) -> String {
-        stamp_values(&self.all_values(), text.as_ref()).1
+        // A value can refer to other values, which a single pass leaves unresolved when the
+        // key it refers to was already handled. Stamp until the text settles, up to the same
+        // limit that finalizing a context uses.
+        let values = self.all_values();
+        let mut result = text.as_ref().to_string();
+        for _ in 0..CONTEXT_FINALIZE_ITERATION_LIMIT {
+            let (changed, stamped) = stamp_values(&values, &result);
+            result = stamped;
+            if !changed || result.len() > CONTEXT_FINALIZE_VALUE_LENGTH_LIMIT {
+                break;
+            }
+        }
+        result
     }
 
     /// Stamps all the items from the iterator `input` with all the values in this [SproutContext]

crates/boot/src/entries.rs

@@ -196,15 +196,12 @@ impl BootableEntry {
     }
 
     /// Create a variant of this entry, with the name suffixed by `suffix` and using `context`.
-    /// The actions of the entry are stamped with `context`, the same as the list generator.
+    /// The actions of the entry are stamped with `context` when the entry is booted.
     /// Variants of the same entry keep the sort key of this entry, so they stay grouped
     /// together in the order they were created in.
     pub fn variant(&self, suffix: &str, context: Rc<SproutContext>) -> Self {
         let mut entry = self.clone();
         entry.name.push_str(suffix);
-        entry.declaration.actions = context
-            .stamp_iter(entry.declaration.actions.iter())
-            .collect();
         // Without any sort key, the name is used to sort, which differs between variants.
         // Pin the sort key to the name of this entry to keep the variants together.
         if entry.sort_key.is_none() && entry.declaration.sort_key.is_none() {

crates/boot/src/generators/list.rs

@@ -20,11 +20,9 @@ pub fn generate(
         context.insert(combination);
         let context = context.freeze();
 
-        // Stamp the entry title and actions from the template.
-        let mut entry = list.entry.clone();
-
-        // Stamp all the actions this entry references.
-        entry.actions = context.stamp_iter(entry.actions.into_iter()).collect();
+        // The actions are stamped with the final context of the entry when it is booted,
+        // so that variants and the values of the entry can override the values of the generator.
+        let entry = list.entry.clone();
 
         // Push the entry into the list with the new context.
         entries.push(BootableEntry::new(

crates/boot/src/main.rs

@@ -506,8 +506,9 @@ fn run(reboot_on_error: &mut bool) -> Result<()> {
     // The preferred entry sources never use an entry with no boot counter tries left.
     // Each source has whether it matches by id, and whether it skips bad entries.
     // In strict mode, the one-shot entry is the default entry for this boot, like in
-    // systemd-boot, and the menu or its timeout still decides what is booted.
-    let oneshot_source = if strict {
+    // systemd-boot, and the menu or its timeout still decides what is booted. This also holds
+    // when the menu is forced, as the one-shot entry is then not booted at once.
+    let oneshot_source = if strict || force_boot_menu {
         bootloader_interface_oneshot_entry.clone()
     } else {
         None
@@ -722,13 +723,29 @@ fn run(reboot_on_error: &mut bool) -> Result<()> {
         // still be marked as good.
         // The tries that were left before this boot used one up, if it did.
         let mut consumed_tries_left = None;
+        let mut entry_context = entry.context();
         if let Some(target) = entry.boot_counter()
             && target.counting
         {
             match target.consume() {
                 Ok(path) => {
                     info!("updated boot counter of entry {}: {}", entry.name(), path);
                     consumed_tries_left = Some(target.counter.tries_left);
+                    // A unified kernel image is the entry file itself, so it has to be booted by
+                    // its new name.
+                    if let Some(chainload) = entry_context.get("chainload")
+                        && let Some(directory) = chainload.len().checked_sub(target.file_name.len())
+                        && chainload
+                            .get(directory..)
+                            .is_some_and(|name| name.eq_ignore_ascii_case(&target.file_name))
+                        && let Some(renamed) =
+                            path.to_string().rsplit('\\').next().map(String::from)
+                    {
+                        let renamed = format!("{}{}", &chainload[..directory], renamed);
+                        let mut context = entry_context.fork();
+                        context.set("chainload", renamed);
+                        entry_context = context.freeze();
+                    }
                     // Tell the system where the counter is, so it can mark the boot as good.
                     let path = edera_sprout_bls::boot_path(&path.to_string());
                     advisory(
@@ -752,8 +769,8 @@ fn run(reboot_on_error: &mut bool) -> Result<()> {
 
         // Execute all the actions for the selected entry.
         for action in &entry.declaration().actions {
-            let action = entry.context().stamp(action);
-            actions::execute(entry.context().clone(), &action)
+            let action = entry_context.stamp(action);
+            actions::execute(entry_context.clone(), &action)
                 .context(format!("unable to execute action '{}'", action))?;
         }
 

crates/eficore/Cargo.toml

@@ -11,7 +11,6 @@ edition.workspace = true
 anyhow.workspace = true
 bitflags.workspace = true
 log.workspace = true
-shlex.workspace = true
 spin.workspace = true
 uefi.workspace = true
 uefi-raw.workspace = true

crates/eficore/src/env.rs

@@ -1,4 +1,4 @@
-use alloc::string::{String, ToString};
+use alloc::string::String;
 use alloc::vec::Vec;
 use anyhow::{Context, Result};
 use uefi::proto::loaded_image::LoadedImage;
@@ -33,14 +33,9 @@ pub fn args() -> Result<Vec<String>> {
         .collect::<Vec<u16>>();
     let options = String::from_utf16_lossy(&options);
 
-    // Use shlex to parse the options.
-    // If shlex fails, we will perform a simple whitespace split.
-    let mut args = shlex::split(&options).unwrap_or_else(|| {
-        options
-            .split_ascii_whitespace()
-            .map(|string| string.to_string())
-            .collect::<Vec<_>>()
-    });
+    // Split the options on whitespace, keeping quoted text together.
+    // Backslashes are kept as they are, since UEFI paths use them as separators.
+    let mut args = split_options(&options);
 
     // Correct firmware that may add invalid arguments at the start.
     // Witnessed this on a Dell Precision 5690 when direct booting.
@@ -66,3 +61,38 @@ pub fn args() -> Result<Vec<String>> {
 
     Ok(args)
 }
+
+/// Splits `options` into arguments on whitespace. Text in single or double quotes is kept together.
+fn split_options(options: &str) -> Vec<String> {
+    let mut args = Vec::new();
+    let mut current = String::new();
+    let mut in_arg = false;
+    let mut quote = None;
+
+    for c in options.chars() {
+        match quote {
+            Some(q) if c == q => quote = None,
+            Some(_) => current.push(c),
+            None if c == '"' || c == '\'' => {
+                quote = Some(c);
+                in_arg = true;
+            }
+            None if c.is_whitespace() => {
+                if in_arg {
+                    args.push(core::mem::take(&mut current));
+                    in_arg = false;
+                }
+            }
+            None => {
+                current.push(c);
+                in_arg = true;
+            }
+        }
+    }
+
+    if in_arg {
+        args.push(current);
+    }
+
+    args
+}