lib-sieve: include: ext-include-common - Change ext_include_get_script() into...

lib-sieve: include: ext-include-common - Change ext_include_get_script() into ext_include_open_script()

Implicitly open the script as well as retrieving it. This fully fixes :optional
sematics. Before, if script object creation succeeded while opening it failed
after all, :optional semantics would still cause an error.

src/lib-sieve/plugins/include/cmd-include.c

@@ -226,7 +226,6 @@ cmd_include_validate(struct sieve_validator *valdtr,
 	struct sieve_script *script;
 	const char *script_name;
 	enum sieve_error error_code = SIEVE_ERROR_NONE;
-	int ret;
 
 	/* Check argument */
 	if (!sieve_validate_positional_argument(valdtr, cmd, arg, "value",
@@ -257,39 +256,17 @@ cmd_include_validate(struct sieve_validator *valdtr,
 		return FALSE;
 	}
 
-	if (ext_include_get_script(this_ext, ctx_data->location,
-				   script_name, &script, &error_code) < 0) {
-		// FIXME: handle ':optional' in this case
-		if (error_code == SIEVE_ERROR_NOT_FOUND) {
-			sieve_argument_validate_error(
-				valdtr, arg, "include: "
-				"included %s script '%s' not found ",
-				ext_include_script_location_name(ctx_data->location),
-				str_sanitize(script_name, 80));
-		} else {
-			sieve_argument_validate_error(
-				valdtr, arg, "include: "
-				"failed to access included %s script '%s' "
-				"(contact system administrator for more information)",
-				ext_include_script_location_name(ctx_data->location),
-				str_sanitize(script_name, 80));
-		}
-		return FALSE;
-	}
-
 	/* Open script */
-	ret = sieve_script_open(script, &error_code);
-	if (ret < 0) {
+	if (ext_include_open_script(this_ext, ctx_data->location,
+				    script_name, &script, &error_code) < 0) {
 		if (error_code != SIEVE_ERROR_NOT_FOUND) {
 			sieve_argument_validate_error(
 				valdtr, arg,
-				"failed to access included %s script '%s': %s",
+				"failed to access included %s script '%s' "
+				"(refer to server log for more information)",
 				ext_include_script_location_name(ctx_data->location),
-				str_sanitize(script_name, 80),
-				sieve_script_get_last_error_lcase(script));
-			sieve_script_unref(&script);
+				str_sanitize(script_name, 80));
 			return FALSE;
-
 		/* Not found */
 		} else {
 			enum sieve_compile_flags cpflags =
@@ -313,7 +290,6 @@ cmd_include_validate(struct sieve_validator *valdtr,
 					"included %s script '%s' does not exist",
 					ext_include_script_location_name(ctx_data->location),
 					str_sanitize(script_name, 80));
-				sieve_script_unref(&script);
 				return FALSE;
 			}
 		}

src/lib-sieve/plugins/include/ext-include-binary.c

@@ -366,16 +366,9 @@ ext_include_binary_open(const struct sieve_extension *ext,
 			return FALSE;
 		}
 
-		/* Can we find the script dependency ? */
-		if (ext_include_get_script(ext, location, str_c(script_name),
-					   &script, &error_code) < 0) {
-			/* No, recompile */
-			// FIXME: handle ':optional' in this case
-			return FALSE;
-		}
-
 		/* Can we open the script dependency ? */
-		if (sieve_script_open(script, &error_code) < 0) {
+		if (ext_include_open_script(ext, location, str_c(script_name),
+					    &script, &error_code) < 0) {
 			if (error_code != SIEVE_ERROR_NOT_FOUND) {
 				/* No, recompile */
 				return FALSE;

src/lib-sieve/plugins/include/ext-include-common.c

@@ -130,11 +130,11 @@ void ext_include_unload(const struct sieve_extension *ext)
  */
 
 static int
-ext_include_get_script_personal(struct sieve_instance *svinst,
-				struct ext_include_context *extctx,
-				const char *script_name,
-				struct sieve_script **script_r,
-				enum sieve_error *error_code_r)
+ext_include_open_script_personal(struct sieve_instance *svinst,
+				 struct ext_include_context *extctx,
+				 const char *script_name,
+				 struct sieve_script **script_r,
+				 enum sieve_error *error_code_r)
 {
 	if (extctx->personal_storage == NULL &&
 	    sieve_storage_create_personal(svinst, NULL, 0,
@@ -142,16 +142,16 @@ ext_include_get_script_personal(struct sieve_instance *svinst,
 					  error_code_r) < 0)
 		return -1;
 
-	return sieve_storage_get_script(extctx->personal_storage, script_name,
-					script_r, error_code_r);
+	return sieve_storage_open_script(extctx->personal_storage, script_name,
+					 script_r, error_code_r);
 }
 
 static int
-ext_include_get_script_global(struct sieve_instance *svinst,
-			      struct ext_include_context *extctx,
-			      const char *script_name,
-			      struct sieve_script **script_r,
-			      enum sieve_error *error_code_r)
+ext_include_open_script_global(struct sieve_instance *svinst,
+			       struct ext_include_context *extctx,
+			       const char *script_name,
+			       struct sieve_script **script_r,
+			       enum sieve_error *error_code_r)
 {
 	if (extctx->global_location == NULL) {
 		e_info(svinst->event, "include: "
@@ -167,35 +167,35 @@ ext_include_get_script_global(struct sieve_instance *svinst,
 				 &extctx->global_storage, error_code_r) < 0)
 		return -1;
 
-	return sieve_storage_get_script(extctx->global_storage, script_name,
-					script_r, error_code_r);
+	return sieve_storage_open_script(extctx->global_storage, script_name,
+					 script_r, error_code_r);
 }
 
-int ext_include_get_script(const struct sieve_extension *ext,
-			   enum ext_include_script_location location,
-			   const char *script_name,
-			   struct sieve_script **script_r,
-			   enum sieve_error *error_code_r)
+int ext_include_open_script(const struct sieve_extension *ext,
+			    enum ext_include_script_location location,
+			    const char *script_name,
+			    struct sieve_script **script_r,
+			    enum sieve_error *error_code_r)
 {
 	struct sieve_instance *svinst = ext->svinst;
 	struct ext_include_context *extctx = ext->context;
 	int ret;
 
+	*script_r = NULL;
 	switch (location) {
 	case EXT_INCLUDE_LOCATION_PERSONAL:
-		ret = ext_include_get_script_personal(svinst, extctx,
-						      script_name,
-						      script_r, error_code_r);
+		ret = ext_include_open_script_personal(svinst, extctx,
+						       script_name,
+						       script_r, error_code_r);
 		break;
 	case EXT_INCLUDE_LOCATION_GLOBAL:
-		ret = ext_include_get_script_global(svinst, extctx,
-						    script_name,
-						    script_r, error_code_r);
+		ret = ext_include_open_script_global(svinst, extctx,
+						     script_name,
+						     script_r, error_code_r);
 		break;
 	default:
 		i_unreached();
 	}
-
 	return ret;
 }
 
@@ -589,7 +589,7 @@ int ext_include_generate_include(
 
 		/* Allocate a new block in the binary and mark the script as
 		   included. */
-		if (script == NULL || !sieve_script_is_open(script)) {
+		if (script == NULL) {
 			/* Just making an empty entry to mark a missing script
 			 */
 			i_assert((flags & EXT_INCLUDE_FLAG_MISSING_AT_UPLOAD) != 0 ||

src/lib-sieve/plugins/include/ext-include-common.h

@@ -82,11 +82,11 @@ extern const struct sieve_operation_def global_operation;
  * Script access
  */
 
-int ext_include_get_script(const struct sieve_extension *ext,
-			   enum ext_include_script_location location,
-			   const char *script_name,
-			   struct sieve_script **script_r,
-			   enum sieve_error *error_code_r);
+int ext_include_open_script(const struct sieve_extension *ext,
+			    enum ext_include_script_location location,
+			    const char *script_name,
+			    struct sieve_script **script_r,
+			    enum sieve_error *error_code_r);
 
 /*
  * Context