Split pnacl and nacl mime types

ppapi/native_client/src/trusted/plugin/json_manifest.cc

Index: ppapi/native_client/src/trusted/plugin/json_manifest.cc
diff --git a/ppapi/native_client/src/trusted/plugin/json_manifest.cc b/ppapi/native_client/src/trusted/plugin/json_manifest.cc
index d1f70ff39f96e278f197b6151e01f34c58b7818b..9c588775abbe0828b6067af4aef8c75f649094cd 100644
--- a/ppapi/native_client/src/trusted/plugin/json_manifest.cc
+++ b/ppapi/native_client/src/trusted/plugin/json_manifest.cc
@@ -45,19 +45,12 @@ const char* const kCacheIdentityKey = "sha256";
 const char* const kOptLevelKey = "-O";
 const char* const kPnaclExperimentalFlags = "experimental_flags";
 
-// Sample manifest file:
+// Sample NaCL manifest file:
 // {
 //   "program": {
 //     "x86-32": {"url": "myprogram_x86-32.nexe"},
 //     "x86-64": {"url": "myprogram_x86-64.nexe"},
-//     "arm": {"url": "myprogram_arm.nexe"},
-//     "portable": {
-//       "pnacl-translate": {
-//         "url": "myprogram.pexe",
-//         "sha256": "...",
-//         "-O": 0
-//       }
-//     }
+//     "arm": {"url": "myprogram_arm.nexe"}
 //   },
 //   "interpreter": {
 //     "x86-32": {"url": "interpreter_x86-32.nexe"},
@@ -73,10 +66,28 @@ const char* const kPnaclExperimentalFlags = "experimental_flags";
 //       "portable": {"url": "bar.txt"}
 //     },
 //     "libfoo.so": {
-//       "x86-64-ivybridge-foo": { "url": "..."},
-//       "x86-64-ivybridge" : { "pnacl-translate": { "url": "..."}},
-//       "x86-64" : { "url": "..." },
-//       "portable": {"pnacl-translate": {"url": "..."}}
+//       "x86-64" : { "url": "..." }
+//     }
+//   }
+// }
+
+// Sample PNaCl manifest file:
+// {
+//   "program": {
+//     "portable": {
+//       "pnacl-translate": {
+//         "url": "myprogram.pexe",
+//         "sha256": "...",
+//         "-O": 0
+//       }
+//     }
+//   },
+//   "files": {
+//     "foo.txt": {
+//       "portable": {"url": "foo.txt"}
+//     },
+//     "bar.txt": {
+//       "portable": {"url": "bar.txt"}
 //     }
 //   }
 // }
@@ -122,7 +133,8 @@ bool IsValidDictionary(const Json::Value& dictionary,
     if (!FindMatchingProperty(property_name,
                               valid_keys,
                               valid_key_count)) {
-      // TODO(jvoung): Should this set error_string and return false?
+      // For forward compatibility, we do not prohibit other keys being in
+      // the dictionary.
       PLUGIN_PRINTF(("WARNING: '%s' property '%s' has unknown key '%s'.\n",
                      parent_key.c_str(),
                      container_key.c_str(), property_name.c_str()));
@@ -147,22 +159,44 @@ bool IsValidDictionary(const Json::Value& dictionary,
 bool IsValidUrlSpec(const Json::Value& url_spec,
                     const nacl::string& container_key,
                     const nacl::string& parent_key,
+                    const nacl::string& sandbox_isa,
                     nacl::string* error_string) {
   static const char* kManifestUrlSpecRequired[] = {
     kUrlKey
   };
-  static const char* kManifestUrlSpecPlusOptional[] = {
-    kUrlKey,
-    kCacheIdentityKey
-  };
+  const char** urlSpecPlusOptional;
+  size_t urlSpecPlusOptionalLength;
+  if (sandbox_isa == kPortableKey) {
+    static const char* kPnaclUrlSpecPlusOptional[] = {
+      kUrlKey,
+      kCacheIdentityKey,
+      kOptLevelKey,
+    };
+    urlSpecPlusOptional = kPnaclUrlSpecPlusOptional;
+    urlSpecPlusOptionalLength = NACL_ARRAY_SIZE(kPnaclUrlSpecPlusOptional);
+  } else {
+    urlSpecPlusOptional = kManifestUrlSpecRequired;
+    urlSpecPlusOptionalLength = NACL_ARRAY_SIZE(kManifestUrlSpecRequired);
+  }
   if (!IsValidDictionary(url_spec, container_key, parent_key,
-                         kManifestUrlSpecPlusOptional,
-                         NACL_ARRAY_SIZE(kManifestUrlSpecPlusOptional),
+                         urlSpecPlusOptional,
+                         urlSpecPlusOptionalLength,
                          kManifestUrlSpecRequired,
                          NACL_ARRAY_SIZE(kManifestUrlSpecRequired),
                          error_string)) {
     return false;
   }
+  // URL specifications must not contain "pnacl-translate" keys.
+  // This prohibits NaCl clients from invoking PNaCl.
+  Json::Value translate = url_spec[kPnaclTranslateKey];
+  if (!translate.empty()) {
+    nacl::stringstream error_stream;
+    error_stream << parent_key << " property '" << container_key <<
+        "' has '" << kPnaclTranslateKey << "' inside URL spec.";
+    *error_string = error_stream.str();
+    return false;
+  }
+  // Verify the correct types of the fields if they exist.
   Json::Value url = url_spec[kUrlKey];
   if (!url.isString()) {
     nacl::stringstream error_stream;
@@ -172,6 +206,24 @@ bool IsValidUrlSpec(const Json::Value& url_spec,
     *error_string = error_stream.str();
     return false;
   }
+  Json::Value cache_identity = url_spec[kCacheIdentityKey];
+  if (!cache_identity.empty() && !cache_identity.isString()) {
+    nacl::stringstream error_stream;
+    error_stream << parent_key << " property '" << container_key <<
+        "' has non-string value '" << cache_identity.toStyledString() <<
+        "' for key '" << kCacheIdentityKey << "'.";
+    *error_string = error_stream.str();
+    return false;
+  }
+  Json::Value opt_level = url_spec[kOptLevelKey];
+  if (!opt_level.empty() && !opt_level.isNumeric()) {
+    nacl::stringstream error_stream;
+    error_stream << parent_key << " property '" << container_key <<
+        "' has non-numeric value '" << opt_level.toStyledString() <<
+        "' for key '" << kOptLevelKey << "'.";
+    *error_string = error_stream.str();
+    return false;
+  }
   return true;
 }
 
@@ -180,6 +232,7 @@ bool IsValidUrlSpec(const Json::Value& url_spec,
 bool IsValidPnaclTranslateSpec(const Json::Value& pnacl_spec,
                                const nacl::string& container_key,
                                const nacl::string& parent_key,
+                               const nacl::string& sandbox_isa,
                                nacl::string* error_string) {
   static const char* kManifestPnaclSpecProperties[] = {
     kPnaclTranslateKey
@@ -194,7 +247,7 @@ bool IsValidPnaclTranslateSpec(const Json::Value& pnacl_spec,
   }
   Json::Value url_spec = pnacl_spec[kPnaclTranslateKey];
   if (!IsValidUrlSpec(url_spec, kPnaclTranslateKey,
-                      container_key, error_string)) {
+                      container_key, sandbox_isa, error_string)) {
     return false;
   }
   return true;
@@ -223,36 +276,75 @@ bool IsValidISADictionary(const Json::Value& dictionary,
   // The keys to the dictionary have to be valid ISA names.
   Json::Value::Members members = dictionary.getMemberNames();
   for (size_t i = 0; i < members.size(); ++i) {
-    // The known ISA values for ISA dictionaries in the manifest.
-    static const char* kManifestISAProperties[] = {
-      kX8632Key,
-      kX8664Key,
-      kArmKey,
-      kPortableKey
-    };
+    const char** isaProperties;
+    size_t isaPropertiesLength;
+    if (sandbox_isa == kPortableKey) {
+      // The known values for PNaCl ISA dictionaries in the manifest.
+      static const char* kPnaclManifestISAProperties[] = {
+        kPortableKey
+      };
+      isaProperties = kPnaclManifestISAProperties;
+      isaPropertiesLength = NACL_ARRAY_SIZE(kPnaclManifestISAProperties);
+    } else {
+      // The known values for NaCl ISA dictionaries in the manifest.
+      static const char* kNaClManifestISAProperties[] = {
+        kX8632Key,
+        kX8664Key,
+        kArmKey,
+        // "portable" is here to allow checking that, if present, it can
+        // only refer to an URL, such as for a data file, and not to
+        // "pnacl-translate", which would cause the creation of a nexe.
+        kPortableKey
+      };
+      isaProperties = kNaClManifestISAProperties;
+      isaPropertiesLength = NACL_ARRAY_SIZE(kNaClManifestISAProperties);
+    }
     nacl::string property_name = members[i];
-    if (!FindMatchingProperty(property_name,
-                              kManifestISAProperties,
-                              NACL_ARRAY_SIZE(kManifestISAProperties))) {
-      PLUGIN_PRINTF(("IsValidISADictionary: unrecognized ISA '%s'.\n",
+    if (FindMatchingProperty(property_name,
+                             isaProperties,
+                             isaPropertiesLength)) {
+      Json::Value property_value = dictionary[property_name];
+      nacl::string error_string;
+      // For NaCl, arch entries can only be
+      //     "arch/portable" : URLSpec
+      // For PNaCl arch in "program" dictionary entries can only be
+      //     "portable" : { "pnacl-translate": URLSpec }
+      // For PNaCl arch elsewhere, dictionary entries can only be
+      //     "portable" : URLSpec
+      if ((sandbox_isa != kPortableKey &&
+           !IsValidUrlSpec(property_value, property_name, parent_key,
+                           sandbox_isa, &error_string)) ||
+          (sandbox_isa == kPortableKey &&
+           parent_key == kProgramKey &&
+           !IsValidPnaclTranslateSpec(property_value, property_name, parent_key,
+                                      sandbox_isa, &error_string)) ||
+          (sandbox_isa == kPortableKey &&
+           parent_key != kProgramKey &&
+           !IsValidUrlSpec(property_value, property_name, parent_key,
+                           sandbox_isa, &error_string))) {
+        error_info->SetReport(ERROR_MANIFEST_SCHEMA_VALIDATE,
+                              nacl::string("manifest: ") + error_string);
+        return false;
+      }
+    } else {
+      // For forward compatibility, we do not prohibit other keys being in
+      // the dictionary.
+      PLUGIN_PRINTF(("IsValidISADictionary: unrecognized key '%s'.\n",
                      property_name.c_str()));
     }
-    // Could be "arch/portable" : URLSpec, or
-    // it could be "arch/portable" : { "pnacl-translate": URLSpec }
-    // for executables that need to be translated.
-    Json::Value property_value = dictionary[property_name];
-    nacl::string error_string;
-    if (!IsValidUrlSpec(property_value, property_name, parent_key,
-                        &error_string) &&
-        !IsValidPnaclTranslateSpec(property_value, property_name,
-                                   parent_key, &error_string)) {
-      error_info->SetReport(ERROR_MANIFEST_SCHEMA_VALIDATE,
-                            nacl::string("manifest: ") + error_string);
-      return false;
-    }
   }
 
-  if (!sandbox_isa.empty()) {
+  if (sandbox_isa == kPortableKey) {
+    bool has_portable = dictionary.isMember(kPortableKey);
+
+    if (!has_portable) {
+      error_info->SetReport(
+          ERROR_MANIFEST_PROGRAM_MISSING_ARCH,
+          nacl::string("manifest: no version of ") + parent_key +
+          " given for portable.");
+      return false;
+    }
+  } else {
     // TODO(elijahtaylor) add ISA resolver here if we expand ISAs to include
     // micro-architectures that can resolve to multiple valid sandboxes.
     bool has_isa = dictionary.isMember(sandbox_isa);
@@ -293,7 +385,6 @@ void GrabUrlAndPnaclOptions(const Json::Value& url_spec,
 bool GetURLFromISADictionary(const Json::Value& dictionary,
                              const nacl::string& parent_key,
                              const nacl::string& sandbox_isa,
-                             bool prefer_portable,
                              nacl::string* url,
                              PnaclOptions* pnacl_options,
                              ErrorInfo* error_info) {
@@ -314,7 +405,7 @@ bool GetURLFromISADictionary(const Json::Value& dictionary,
   bool has_portable = dictionary.isMember(kPortableKey);
   bool has_isa = dictionary.isMember(sandbox_isa);
   nacl::string chosen_isa;
-  if ((has_portable && prefer_portable) || !has_isa) {
+  if ((sandbox_isa == kPortableKey) || (has_portable && !has_isa)) {
     chosen_isa = kPortableKey;
   } else {
     chosen_isa = sandbox_isa;
@@ -323,9 +414,11 @@ bool GetURLFromISADictionary(const Json::Value& dictionary,
   // Check if this requires a pnacl-translate, otherwise just grab the URL.
   // We may have pnacl-translate for isa-specific bitcode for CPU tuning.
   if (isa_spec.isMember(kPnaclTranslateKey)) {
+    // PNaCl
     GrabUrlAndPnaclOptions(isa_spec[kPnaclTranslateKey], url, pnacl_options);
     pnacl_options->set_translate(true);
   } else {
+    // NaCl
     *url = isa_spec[kUrlKey].asString();
     pnacl_options->set_translate(false);
   }
@@ -337,7 +430,6 @@ bool GetKeyUrl(const Json::Value& dictionary,
                const nacl::string& key,
                const nacl::string& sandbox_isa,
                const Manifest* manifest,
-               bool prefer_portable,
                nacl::string* full_url,
                PnaclOptions* pnacl_options,
                ErrorInfo* error_info) {
@@ -349,8 +441,8 @@ bool GetKeyUrl(const Json::Value& dictionary,
   }
   const Json::Value& isa_dict = dictionary[key];
   nacl::string relative_url;
-  if (!GetURLFromISADictionary(isa_dict, key, sandbox_isa, prefer_portable,
-                               &relative_url, pnacl_options, error_info)) {
+  if (!GetURLFromISADictionary(isa_dict, key, sandbox_isa, &relative_url,
+                               pnacl_options, error_info)) {
     return false;
   }
   return manifest->ResolveURL(relative_url, full_url, error_info);
@@ -440,7 +532,7 @@ bool JsonManifest::MatchesSchema(ErrorInfo* error_info) {
       nacl::string file_name = members[i];
       if (!IsValidISADictionary(files[file_name],
                                 file_name,
-                                nacl::string(),
+                                sandbox_isa_,
                                 error_info)) {
         return false;
       }
@@ -484,7 +576,6 @@ bool JsonManifest::GetProgramURL(nacl::string* full_url,
   if (!GetURLFromISADictionary(program,
                                kProgramKey,
                                sandbox_isa_,
-                               prefer_portable_,
                                &nexe_url,
                                pnacl_options,
                                error_info)) {
@@ -519,8 +610,8 @@ bool JsonManifest::ResolveKey(const nacl::string& key,
     return false;
 
   if (key == kProgramKey) {
-    return GetKeyUrl(dictionary_, key, sandbox_isa_, this, prefer_portable_,
-                     full_url, pnacl_options, error_info);
+    return GetKeyUrl(dictionary_, key, sandbox_isa_, this, full_url,
+                     pnacl_options, error_info);
   }
   nacl::string::const_iterator p = find(key.begin(), key.end(), '/');
   if (p == key.end()) {
@@ -554,8 +645,8 @@ bool JsonManifest::ResolveKey(const nacl::string& key,
         nacl::string("ResolveKey: no such \"files\" entry: ") + key);
     return false;
   }
-  return GetKeyUrl(files, rest, sandbox_isa_, this, prefer_portable_,
-                   full_url, pnacl_options, error_info);
+  return GetKeyUrl(files, rest, sandbox_isa_, this, full_url, pnacl_options,
+                   error_info);
 }
 
 }  // namespace plugin