Repository: bytecodealliance/wasm-tools
Pull request #1473 (2024-03-27 15:31:58)

Bail when doing `wit_component::decode` on a component
Ran into an issue where I was accidentally calling `wasm-tools component new` on a wasm binary that was already a component. Unfortunately, there's nothing that prevents this, and things only blow up deep inside the parser where an EOF error is returned. I believe there's no good reason for calling `wit_component::decode` with a component so this feels like the right place to return this error.

alexcrichton: Nice! Mind adding a CLI test as well for this? Dropping something in `tests/cli/*.wat` should suffice where it contains `(component)` and tries to do a `component new` with the result.

Patch:
diff --git a/crates/wit-component/src/metadata.rs b/crates/wit-component/src/metadata.rs
index 2409b02e..4c7fcf81 100644
--- a/crates/wit-component/src/metadata.rs
+++ b/crates/wit-component/src/metadata.rs
@@ -50,7 +50,7 @@ use wasm_encoder::{
     ComponentBuilder, ComponentExportKind, ComponentType, ComponentTypeRef, CustomSection,
 };
 use wasm_metadata::Producers;
-use wasmparser::{BinaryReader, Parser, Payload};
+use wasmparser::{BinaryReader, Encoding, Parser, Payload};
 use wit_parser::{Package, PackageName, Resolve, World, WorldId, WorldItem};
 
 const CURRENT_VERSION: u8 = 0x04;
@@ -127,6 +127,8 @@ pub struct ModuleMetadata {
 /// The extraction here provides the metadata necessary to continue the process
 /// later on.
 ///
+/// This will return an error if `wasm` is not a valid WebAssembly module.
+///
 /// Note that a "stripped" binary where `component-type` sections are removed
 /// is returned as well to embed within a component.
 pub fn decode(wasm: &[u8]) -> Result<(Vec<u8>, Bindgen)> {
@@ -142,6 +144,9 @@ pub fn decode(wasm: &[u8]) -> Result<(Vec<u8>, Bindgen)> {
                 ret.merge(data)
                     .with_context(|| format!("updating metadata for section {}", cs.name()))?;
             }
+            wasmparser::Payload::Version { encoding, .. } if encoding != Encoding::Module => {
+                bail!("decoding a component is not supported")
+            }
             _ => {
                 if let Some((id, range)) = payload.as_section() {
                     new_module.section(&wasm_encoder::RawSection {
diff --git a/crates/wit-smith/src/generate.rs b/crates/wit-smith/src/generate.rs
index bbcbc302..298979af 100644
--- a/crates/wit-smith/src/generate.rs
+++ b/crates/wit-smith/src/generate.rs
@@ -318,16 +318,14 @@ impl Generator {
 
 impl<'a> InterfaceGenerator<'a> {
     fn new(gen: &'a Generator, file: &'a mut File) -> InterfaceGenerator<'a> {
-        // Claim the name `memory` to avoid conflicting with the canonical ABI
-        // always using a linear memory named `memory`.
-        let mut unique_names = HashSet::new();
-        unique_names.insert("memory".to_string());
         InterfaceGenerator {
             gen,
             file,
             config: &gen.config,
             types_in_interface: Vec::new(),
-            unique_names,
+            // Claim the name `memory` to avoid conflicting with the canonical
+            // ABI always using a linear memory named `memory`.
+            unique_names: HashSet::from_iter(["memory".to_string()]),
         }
     }
 
@@ -413,7 +411,10 @@ impl<'a> InterfaceGenerator<'a> {
         let mut parts = Vec::new();
         let mut imported_interfaces = HashSet::new();
         let mut exported_interfaces = HashSet::new();
-        let mut export_names = HashSet::new();
+
+        // Claim the name `memory` to avoid conflicting with the canonical
+        // ABI always using a linear memory named `memory`.
+        let mut export_names = HashSet::from_iter(["memory".to_string()]);
 
         while parts.len() < self.config.max_world_items && !u.is_empty() && u.arbitrary()? {
             let kind = u.arbitrary::<ItemKind>()?;


Test patch:
diff --git a/tests/cli/component-new-existing-component.wat b/tests/cli/component-new-existing-component.wat
new file mode 100644
index 00000000..2251e2ef
--- /dev/null
+++ b/tests/cli/component-new-existing-component.wat
@@ -0,0 +1,2 @@
+;; FAIL: component new % -o component.wasm
+(component)
diff --git a/tests/cli/component-new-existing-component.wat.stderr b/tests/cli/component-new-existing-component.wat.stderr
new file mode 100644
index 00000000..bb6a3987
--- /dev/null
+++ b/tests/cli/component-new-existing-component.wat.stderr
@@ -0,0 +1,1 @@
+error: decoding a component is not supported