Use file magic to determine file type, not file extension.

Bug: 10614658
Change-Id: I9156dfca78ac8cd1c62fb258825cc791629270a4
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 800fab0..5728bf6 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -794,7 +794,7 @@
 const DexFile* ClassLinker::FindOrCreateOatFileForDexLocationLocked(const std::string& dex_location,
                                                                     const std::string& oat_location) {
   uint32_t dex_location_checksum;
-  if (!DexFile::GetChecksum(dex_location, dex_location_checksum)) {
+  if (!DexFile::GetChecksum(dex_location, &dex_location_checksum)) {
     LOG(ERROR) << "Failed to compute checksum '" << dex_location << "'";
     return NULL;
   }
@@ -915,7 +915,7 @@
   UniquePtr<const OatFile> oat_file(FindOatFileFromOatLocationLocked(odex_filename));
   if (oat_file.get() != NULL) {
     uint32_t dex_location_checksum;
-    if (!DexFile::GetChecksum(dex_location, dex_location_checksum)) {
+    if (!DexFile::GetChecksum(dex_location, &dex_location_checksum)) {
       // If no classes.dex found in dex_location, it has been stripped, assume oat is up-to-date.
       // This is the common case in user builds for jar's and apk's in the /system directory.
       const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_location);
@@ -936,7 +936,7 @@
   oat_file.reset(FindOatFileFromOatLocationLocked(cache_location));
   if (oat_file.get() != NULL) {
     uint32_t dex_location_checksum;
-    if (!DexFile::GetChecksum(dex_location, dex_location_checksum)) {
+    if (!DexFile::GetChecksum(dex_location, &dex_location_checksum)) {
       LOG(WARNING) << "Failed to compute checksum: " << dex_location;
       return NULL;
     }
@@ -1932,7 +1932,8 @@
 void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file, SirtRef<mirror::DexCache>& dex_cache) {
   dex_lock_.AssertExclusiveHeld(Thread::Current());
   CHECK(dex_cache.get() != NULL) << dex_file.GetLocation();
-  CHECK(dex_cache->GetLocation()->Equals(dex_file.GetLocation()));
+  CHECK(dex_cache->GetLocation()->Equals(dex_file.GetLocation()))
+      << dex_cache->GetLocation()->ToModifiedUtf8() << " " << dex_file.GetLocation();
   dex_caches_.push_back(dex_cache.get());
   dex_cache->SetDexFile(&dex_file);
   dex_caches_dirty_ = true;
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index e81c456..9034628 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -62,9 +62,34 @@
                         reinterpret_cast<const DexFile::ClassDef*>(NULL));
 }
 
-bool DexFile::GetChecksum(const std::string& filename, uint32_t& checksum) {
-  if (IsValidZipFilename(filename)) {
-    UniquePtr<ZipArchive> zip_archive(ZipArchive::Open(filename));
+int OpenAndReadMagic(const std::string& filename, uint32_t* magic) {
+  CHECK(magic != NULL);
+  int fd = open(filename.c_str(), O_RDONLY, 0);
+  if (fd == -1) {
+    PLOG(WARNING) << "Unable to open '" << filename << "'";
+    return -1;
+  }
+  int n = TEMP_FAILURE_RETRY(read(fd, magic, sizeof(*magic)));
+  if (n != sizeof(*magic)) {
+    PLOG(ERROR) << "Failed to find magic in '" << filename << "'";
+    return -1;
+  }
+  if (lseek(fd, 0, SEEK_SET) != 0) {
+    PLOG(ERROR) << "Failed to seek to beginning of file '" << filename << "'";
+    return -1;
+  }
+  return fd;
+}
+
+bool DexFile::GetChecksum(const std::string& filename, uint32_t* checksum) {
+  CHECK(checksum != NULL);
+  uint32_t magic;
+  int fd = OpenAndReadMagic(filename, &magic);
+  if (fd == -1) {
+    return false;
+  }
+  if (IsZipMagic(magic)) {
+    UniquePtr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd));
     if (zip_archive.get() == NULL) {
       return false;
     }
@@ -73,30 +98,36 @@
       LOG(ERROR) << "Zip archive '" << filename << "' doesn't contain " << kClassesDex;
       return false;
     }
-    checksum = zip_entry->GetCrc32();
+    *checksum = zip_entry->GetCrc32();
     return true;
   }
-  if (IsValidDexFilename(filename)) {
-    UniquePtr<const DexFile> dex_file(DexFile::OpenFile(filename, filename, false));
+  if (IsDexMagic(magic)) {
+    UniquePtr<const DexFile> dex_file(DexFile::OpenFile(fd, filename, false));
     if (dex_file.get() == NULL) {
       return false;
     }
-    checksum = dex_file->GetHeader().checksum_;
+    *checksum = dex_file->GetHeader().checksum_;
     return true;
   }
-  LOG(ERROR) << "Expected valid zip or dex file name: " << filename;
+  LOG(ERROR) << "Expected valid zip or dex file: " << filename;
   return false;
 }
 
 const DexFile* DexFile::Open(const std::string& filename,
                              const std::string& location) {
-  if (IsValidZipFilename(filename)) {
-    return DexFile::OpenZip(filename, location);
+  uint32_t magic;
+  int fd = OpenAndReadMagic(filename, &magic);
+  if (fd == -1) {
+    return NULL;
   }
-  if (!IsValidDexFilename(filename)) {
-    LOG(WARNING) << "Attempting to open dex file with unknown extension '" << filename << "'";
+  if (IsZipMagic(magic)) {
+    return DexFile::OpenZip(fd, location);
   }
-  return DexFile::OpenFile(filename, location, true);
+  if (IsDexMagic(magic)) {
+    return DexFile::OpenFile(fd, location, true);
+  }
+  LOG(ERROR) << "Expected valid zip or dex file: " << filename;
+  return NULL;
 }
 
 int DexFile::GetPermissions() const {
@@ -129,37 +160,32 @@
   }
 }
 
-const DexFile* DexFile::OpenFile(const std::string& filename,
+const DexFile* DexFile::OpenFile(int fd,
                                  const std::string& location,
                                  bool verify) {
-  CHECK(!location.empty()) << filename;
-  int fd = open(filename.c_str(), O_RDONLY);  // TODO: scoped_fd
-  if (fd == -1) {
-    PLOG(ERROR) << "open(\"" << filename << "\", O_RDONLY) failed";
-    return NULL;
-  }
+  CHECK(!location.empty());
   struct stat sbuf;
   memset(&sbuf, 0, sizeof(sbuf));
   if (fstat(fd, &sbuf) == -1) {
-    PLOG(ERROR) << "fstat \"" << filename << "\" failed";
+    PLOG(ERROR) << "fstat \"" << location << "\" failed";
     close(fd);
     return NULL;
   }
   if (S_ISDIR(sbuf.st_mode)) {
-    LOG(ERROR) << "attempt to mmap directory \"" << filename << "\"";
+    LOG(ERROR) << "attempt to mmap directory \"" << location << "\"";
     return NULL;
   }
   size_t length = sbuf.st_size;
   UniquePtr<MemMap> map(MemMap::MapFile(length, PROT_READ, MAP_PRIVATE, fd, 0));
   if (map.get() == NULL) {
-    LOG(ERROR) << "mmap \"" << filename << "\" failed";
+    LOG(ERROR) << "mmap \"" << location << "\" failed";
     close(fd);
     return NULL;
   }
   close(fd);
 
   if (map->Size() < sizeof(DexFile::Header)) {
-    LOG(ERROR) << "Failed to open dex file '" << filename << "' that is too short to have a header";
+    LOG(ERROR) << "Failed to open dex file '" << location << "' that is too short to have a header";
     return NULL;
   }
 
@@ -167,12 +193,12 @@
 
   const DexFile* dex_file = OpenMemory(location, dex_header->checksum_, map.release());
   if (dex_file == NULL) {
-    LOG(ERROR) << "Failed to open dex file '" << filename << "' from memory";
+    LOG(ERROR) << "Failed to open dex file '" << location << "' from memory";
     return NULL;
   }
 
   if (verify && !DexFileVerifier::Verify(dex_file, dex_file->Begin(), dex_file->Size())) {
-    LOG(ERROR) << "Failed to verify dex file '" << filename << "'";
+    LOG(ERROR) << "Failed to verify dex file '" << location << "'";
     return NULL;
   }
 
@@ -181,11 +207,10 @@
 
 const char* DexFile::kClassesDex = "classes.dex";
 
-const DexFile* DexFile::OpenZip(const std::string& filename,
-                                const std::string& location) {
-  UniquePtr<ZipArchive> zip_archive(ZipArchive::Open(filename));
+const DexFile* DexFile::OpenZip(int fd, const std::string& location) {
+  UniquePtr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd));
   if (zip_archive.get() == NULL) {
-    LOG(ERROR) << "Failed to open " << filename << " when looking for classes.dex";
+    LOG(ERROR) << "Failed to open " << location << " when looking for classes.dex";
     return NULL;
   }
   return DexFile::Open(*zip_archive.get(), location);
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 7be5cb8..346154c 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -346,7 +346,7 @@
   // For .dex files, this is the header checksum.
   // For zip files, this is the classes.dex zip entry CRC32 checksum.
   // Return true if the checksum could be found, false otherwise.
-  static bool GetChecksum(const std::string& filename, uint32_t& checksum)
+  static bool GetChecksum(const std::string& filename, uint32_t* checksum)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   // Opens .dex file, guessing the container format based on file extension
@@ -815,13 +815,12 @@
 
  private:
   // Opens a .dex file
-  static const DexFile* OpenFile(const std::string& filename,
+  static const DexFile* OpenFile(int fd,
                                  const std::string& location,
                                  bool verify);
 
   // Opens a dex file from within a .jar, .zip, or .apk file
-  static const DexFile* OpenZip(const std::string& filename,
-                                const std::string& location);
+  static const DexFile* OpenZip(int fd, const std::string& location);
 
   // Opens a .dex file at the given address backed by a MemMap
   static const DexFile* OpenMemory(const std::string& location,
diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc
index 32a8354..272c42d 100644
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -120,7 +120,7 @@
 TEST_F(DexFileTest, GetChecksum) {
   uint32_t checksum;
   ScopedObjectAccess soa(Thread::Current());
-  EXPECT_TRUE(DexFile::GetChecksum(GetLibCoreDexFileName(), checksum));
+  EXPECT_TRUE(DexFile::GetChecksum(GetLibCoreDexFileName(), &checksum));
   EXPECT_EQ(java_lang_dex_file_->GetLocationChecksum(), checksum);
 }
 
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index e2e5bf7..1cd33ee 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -276,7 +276,7 @@
   for (const OatFile::OatDexFile* oat_dex_file : oat_file_->GetOatDexFiles()) {
     const std::string& dex_file_location = oat_dex_file->GetDexFileLocation();
     uint32_t dex_file_location_checksum;
-    if (!DexFile::GetChecksum(dex_file_location.c_str(), dex_file_location_checksum)) {
+    if (!DexFile::GetChecksum(dex_file_location.c_str(), &dex_file_location_checksum)) {
       LOG(WARNING) << "ValidateOatFile could not find checksum for " << dex_file_location;
       return false;
     }
diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc
index d2a6c0e..4c60b8f 100644
--- a/runtime/native/dalvik_system_DexFile.cc
+++ b/runtime/native/dalvik_system_DexFile.cc
@@ -228,7 +228,7 @@
     } else {
       uint32_t location_checksum;
       // If we have no classes.dex checksum such as in a user build, assume up-to-date.
-      if (!DexFile::GetChecksum(filename.c_str(), location_checksum)) {
+      if (!DexFile::GetChecksum(filename.c_str(), &location_checksum)) {
         if (debug_logging) {
           LOG(INFO) << "DexFile_isDexOptNeeded ignoring precompiled stripped file: "
               << filename.c_str();
@@ -278,7 +278,7 @@
 
   ScopedObjectAccess soa(env);
   uint32_t location_checksum;
-  if (!DexFile::GetChecksum(filename.c_str(), location_checksum)) {
+  if (!DexFile::GetChecksum(filename.c_str(), &location_checksum)) {
     LOG(ERROR) << "DexFile_isDexOptNeeded failed to compute checksum of " << filename.c_str();
     return JNI_TRUE;
   }
diff --git a/runtime/oat.h b/runtime/oat.h
index a5c6bed..a653cf8 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -27,6 +27,9 @@
 
 class PACKED(4) OatHeader {
  public:
+  static const uint8_t kOatMagic[4];
+  static const uint8_t kOatVersion[4];
+
   OatHeader();
   OatHeader(InstructionSet instruction_set,
             const std::vector<const DexFile*>* dex_files,
@@ -78,9 +81,6 @@
   std::string GetImageFileLocation() const;
 
  private:
-  static const uint8_t kOatMagic[4];
-  static const uint8_t kOatVersion[4];
-
   uint8_t magic_[4];
   uint8_t version_[4];
   uint32_t adler32_checksum_;
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 4c97017..abd1287 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -33,19 +33,18 @@
 namespace art {
 
 std::string OatFile::DexFilenameToOdexFilename(const std::string& location) {
-  CHECK(IsValidDexFilename(location) || IsValidZipFilename(location));
+  CHECK_GE(location.size(), 4U) << location;  // must be at least .123
+  size_t dot_index = location.size() - 3 - 1;  // 3=dex or zip or apk
+  CHECK_EQ('.', location[dot_index]) << location;
   std::string odex_location(location);
-  odex_location.resize(odex_location.size() - 3);  // 3=dex or zip or apk
-  CHECK_EQ('.', odex_location[odex_location.size()-1]);
+  odex_location.resize(dot_index + 1);
+  CHECK_EQ('.', odex_location[odex_location.size()-1]) << location << " " << odex_location;
   odex_location += "odex";
   return odex_location;
 }
 
 void OatFile::CheckLocation(const std::string& location) {
   CHECK(!location.empty());
-  if (!IsValidOatFilename(location)) {
-    LOG(WARNING) << "Attempting to open oat file with unknown extension '" << location << "'";
-  }
 }
 
 OatFile* OatFile::OpenMemory(std::vector<uint8_t>& oat_contents,
diff --git a/runtime/utils.cc b/runtime/utils.cc
index dcfe8a7..8e810a7 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -1196,7 +1196,7 @@
     LOG(FATAL) << "Expected path in location to be absolute: "<< location;
   }
   std::string cache_file(location, 1);  // skip leading slash
-  if (!IsValidDexFilename(location) && !IsValidImageFilename(location)) {
+  if (!EndsWith(location, ".dex") || !EndsWith(location, ".art")) {
     cache_file += "/";
     cache_file += DexFile::kClassesDex;
   }
@@ -1204,27 +1204,19 @@
   return dalvik_cache + "/" + cache_file;
 }
 
-bool IsValidZipFilename(const std::string& filename) {
-  if (filename.size() < 4) {
-    return false;
-  }
-  std::string suffix(filename.substr(filename.size() - 4));
-  return (suffix == ".zip" || suffix == ".jar" || suffix == ".apk");
+bool IsZipMagic(uint32_t magic) {
+  return (('P' == ((magic >> 0) & 0xff)) &&
+          ('K' == ((magic >> 8) & 0xff)));
 }
 
-bool IsValidDexFilename(const std::string& filename) {
-  return EndsWith(filename, ".dex");
+bool IsDexMagic(uint32_t magic) {
+  return DexFile::IsMagicValid(reinterpret_cast<const byte*>(&magic));
 }
 
-bool IsValidImageFilename(const std::string& filename) {
-  return EndsWith(filename, ".art");
-}
-
-bool IsValidOatFilename(const std::string& filename) {
-  return (EndsWith(filename, ".odex") ||
-          EndsWith(filename, ".dex") ||
-          EndsWith(filename, ".oat") ||
-          EndsWith(filename, DexFile::kClassesDex));
+bool IsOatMagic(uint32_t magic) {
+  return (memcmp(reinterpret_cast<const byte*>(magic),
+                 OatHeader::kOatMagic,
+                 sizeof(OatHeader::kOatMagic)) == 0);
 }
 
 }  // namespace art
diff --git a/runtime/utils.h b/runtime/utils.h
index c506fba..812a581 100644
--- a/runtime/utils.h
+++ b/runtime/utils.h
@@ -353,11 +353,10 @@
 // Returns the dalvik-cache location for a DexFile or OatFile, or dies trying.
 std::string GetDalvikCacheFilenameOrDie(const std::string& location);
 
-// Check whether the given filename has a valid extension
-bool IsValidZipFilename(const std::string& filename);
-bool IsValidDexFilename(const std::string& filename);
-bool IsValidImageFilename(const std::string& filename);
-bool IsValidOatFilename(const std::string& filename);
+// Check whether the given magic matches a known file type.
+bool IsZipMagic(uint32_t magic);
+bool IsDexMagic(uint32_t magic);
+bool IsOatMagic(uint32_t magic);
 
 class VoidFunctor {
  public:
diff --git a/runtime/zip_archive.cc b/runtime/zip_archive.cc
index c3167e5..8e09e78 100644
--- a/runtime/zip_archive.cc
+++ b/runtime/zip_archive.cc
@@ -336,11 +336,11 @@
     PLOG(WARNING) << "Unable to open '" << filename << "'";
     return NULL;
   }
-  SetCloseOnExec(fd);
   return OpenFromFd(fd);
 }
 
 ZipArchive* ZipArchive::OpenFromFd(int fd) {
+  SetCloseOnExec(fd);
   UniquePtr<ZipArchive> zip_archive(new ZipArchive(fd));
   if (zip_archive.get() == NULL) {
       return NULL;