add errors to incorrect version installation as well

duckdb · Mytherin · May 21, 2024 · Jan 2, 2024 · Feb 2, 2024 · Feb 2, 2024
commit 2f294b17edfbd77f161976b12d4ba9b0ea54ad75
diff --git a/src/include/duckdb/main/extension.hpp b/src/include/duckdb/main/extension.hpp
@@ -40,6 +40,9 @@ struct ParsedExtensionMetaData {
 	bool AppearsValid() {
 		return magic_value == EXPECTED_MAGIC_VALUE;
 	}
+
+	// Returns an error string describing which parts of the metadata are mismatcheds
+	string GetInvalidMetadataError();
 };
 
 } // namespace duckdb
diff --git a/src/main/extension.cpp b/src/main/extension.cpp
@@ -1,8 +1,63 @@
 #include "duckdb/main/extension.hpp"
+#include "duckdb/common/string_util.hpp"
+#include "duckdb/main/extension_helper.hpp"
+
 
 namespace duckdb {
 
 Extension::~Extension() {
 }
 
+static string PrettyPrintString(const string &s) {
+	string res = "";
+	for (auto c : s) {
+		if (StringUtil::CharacterIsAlpha(c) || StringUtil::CharacterIsDigit(c) || c == '_' || c == '-' || c == ' ' ||
+		    c == '.') {
+			res += c;
+		} else {
+			uint8_t value = c;
+			res += "\\x";
+			uint8_t first = value / 16;
+			if (first < 10) {
+				res.push_back((char)('0' + first));
+			} else {
+				res.push_back((char)('a' + first - 10));
+			}
+			uint8_t second = value % 16;
+			if (second < 10) {
+				res.push_back((char)('0' + second));
+			} else {
+				res.push_back((char)('a' + second - 10));
+			}
+		}
+	}
+	return res;
+}
+
+string ParsedExtensionMetaData::GetInvalidMetadataError() {
+	const string engine_version = string(ExtensionHelper::GetVersionDirectoryName());
+	const string engine_platform = string(DuckDB::Platform());
+
+	if (!AppearsValid()) {
+		return "The file is not a DuckDB extension. The metadata at the end of the file is invalid";
+	}
+
+	string result;
+	if (engine_version != duckdb_version) {
+		result += StringUtil::Format("The file was built for DuckDB version '%s', but we can only load extensions built for DuckDB version '%s'.",
+		                       PrettyPrintString(duckdb_version), engine_version);
+	}
+	if (engine_platform != platform) {
+		if (!result.empty()) {
+			result += "Also, t";
+		} else {
+			result += "T";
+		}
+		result += StringUtil::Format("he file was built for the platform '%s', but we can only load extensions built for platform '%s'.",
+		                              PrettyPrintString(platform), engine_platform);
+	}
+
+	return result;
+}
+
 } // namespace duckdb
diff --git a/src/main/extension/extension_install.cpp b/src/main/extension/extension_install.cpp
@@ -232,6 +232,23 @@ string ExtensionHelper::ExtensionFinalizeUrlTemplate(const string &url_template,
 	return url;
 }
 
+static void CheckExtensionMetadataOnInstall(DBConfig &config, void *in_buffer, idx_t file_size, ExtensionInstallInfo &info, const string &extension_name) {
+	if (file_size < ParsedExtensionMetaData::FOOTER_SIZE) {
+		throw IOException("Failed to install '%s', file too small to be a valid DuckDB extension!", extension_name);
+	}
+
+	auto parsed_metadata = ExtensionHelper::ParseExtensionMetaData(static_cast<char *>(in_buffer) +
+	                                                               (file_size - ParsedExtensionMetaData::FOOTER_SIZE));
+
+	auto metadata_mismatch_error = parsed_metadata.GetInvalidMetadataError();
+
+	if (!metadata_mismatch_error.empty() && !config.options.allow_extensions_metadata_mismatch) {
+		throw IOException("Failed to install '%s', %s", extension_name, metadata_mismatch_error);
+	}
+
+	info.version = parsed_metadata.extension_version;
+}
+
 static void WriteExtensionFiles(FileSystem &fs, const string &temp_path, const string &local_extension_path,
                                 void *in_buffer, idx_t file_size, bool force_install, ExtensionInstallInfo &info) {
 	// Write files
@@ -242,13 +259,6 @@ static void WriteExtensionFiles(FileSystem &fs, const string &temp_path, const s
 	}
 	fs.MoveFile(temp_path, local_extension_path);
 
-	// This is the moment to parse the metadata to get the version info from the buffer
-	auto parsed_metadata = ExtensionHelper::ParseExtensionMetaData(static_cast<char *>(in_buffer) +
-	                                                               (file_size - ParsedExtensionMetaData::FOOTER_SIZE));
-	if (parsed_metadata.AppearsValid()) {
-		info.version = parsed_metadata.extension_version;
-	}
-
 	// Write metadata
 	auto metadata_tmp_path = temp_path + ".info";
 	auto metadata_file_path = local_extension_path + ".info";
@@ -281,6 +291,7 @@ static unique_ptr<ExtensionInstallInfo> DirectInstallExtension(DBConfig &config,
 	auto in_buffer = ReadExtensionFileFromDisk(fs, file, file_size);
 
 	ExtensionInstallInfo info;
+	CheckExtensionMetadataOnInstall(config, (void *)in_buffer.get(), file_size, info, extension_name);
 
 	if (repository_url.empty()) {
 		info.mode = ExtensionInstallMode::CUSTOM_PATH;
@@ -337,6 +348,8 @@ static unique_ptr<ExtensionInstallInfo> InstallFromHttpUrl(DBConfig &config, con
 	auto decompressed_body = GZipFileSystem::UncompressGZIPString(res->body);
 
 	ExtensionInstallInfo info;
+	CheckExtensionMetadataOnInstall(config, (void *)decompressed_body.data(), decompressed_body.size(), info, extension_name);
+
 	info.mode = ExtensionInstallMode::REPOSITORY;
 	info.full_path = url;
 	info.repository_url = repository_url;

diff --git a/src/main/extension/extension_load.cpp b/src/main/extension/extension_load.cpp
@@ -65,32 +65,6 @@ static string FilterZeroAtEnd(string s) {
 	return s;
 }
 
-static string PrettyPrintString(const string &s) {
-	string res = "";
-	for (auto c : s) {
-		if (StringUtil::CharacterIsAlpha(c) || StringUtil::CharacterIsDigit(c) || c == '_' || c == '-' || c == ' ' ||
-		    c == '.') {
-			res += c;
-		} else {
-			uint8_t value = c;
-			res += "\\x";
-			uint8_t first = value / 16;
-			if (first < 10) {
-				res.push_back((char)('0' + first));
-			} else {
-				res.push_back((char)('a' + first - 10));
-			}
-			uint8_t second = value % 16;
-			if (second < 10) {
-				res.push_back((char)('0' + second));
-			} else {
-				res.push_back((char)('a' + second - 10));
-			}
-		}
-	}
-	return res;
-}
-
 ParsedExtensionMetaData ExtensionHelper::ParseExtensionMetaData(const char *metadata) {
 	ParsedExtensionMetaData result;
 
@@ -250,32 +224,12 @@ bool ExtensionHelper::TryInitialLoad(DBConfig &config, FileSystem &fs, const str
 	// Parse the extension metadata from the extension binary
 	auto parsed_metadata = ParseExtensionMetaData(*handle);
 
-	string metadata_mismatch_error;
-	const string engine_version = string(ExtensionHelper::GetVersionDirectoryName());
-	const string engine_platform = string(DuckDB::Platform());
+	auto metadata_mismatch_error = parsed_metadata.GetInvalidMetadataError();
 
-	// Check the parsed
-	if (!parsed_metadata.AppearsValid()) {
-		// metadata do not looks right, add this to the error message
-		metadata_mismatch_error = StringUtil::Format("\nFile '%s' is not a DuckDB extension. The metadata at the end of the file is invalid. "
-		                              "This version of DuckDB can only load extensions that are compiled for DuckDB "
-		                              "version '%s', platform '%s'.", handle->path, engine_version, engine_platform);
-	} else if (engine_version != parsed_metadata.duckdb_version ||  engine_platform != parsed_metadata.platform) {
-		metadata_mismatch_error = StringUtil::Format("\nFile '%s' is a DuckDB extension, but the metadata does not match:", handle->path);
-
-		if (engine_version != parsed_metadata.duckdb_version) {
-			metadata_mismatch_error +=
-			    "\n" + StringUtil::Format("\nThe file was built for DuckDB version '%s', but we can only load extensions built for DuckDB version '%s'.",
-			                               PrettyPrintString(parsed_metadata.duckdb_version), engine_version);
-		}
-		if (engine_platform != parsed_metadata.platform) {
-			metadata_mismatch_error +=
-			    "\n" + StringUtil::Format("\nThe file was built for the platform '%s', but we can only load extensions built for platform '%s'.",
-			                             PrettyPrintString(parsed_metadata.platform), engine_platform);
-		}
+	if (!metadata_mismatch_error.empty()) {
+		metadata_mismatch_error = StringUtil::Format("Failed to load '%s'",extension) + "\n" + metadata_mismatch_error;
 	}
 
-
 	if (!config.options.allow_unsigned_extensions) {
 		bool signature_valid = CheckExtensionSignature(*handle, parsed_metadata);