Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5b54d7bd authored by Mark Punzalan's avatar Mark Punzalan
Browse files

Revert "[res] Optimize Idmap verification - use mtime as crc" 

This reverts commit 1ebcbc67.

This is a partial revert; the aconfig flag definition for `android.content.res.idmap_crc_is_mtime` and `*.bp` changes are kept

Reason for revert: framework-res.apk idmaps don't get regenerated when they should be, since mtime is normalized to the same timestamp across Pixel builds

Bug: b/433857121
Bug: b/422474410
Change-Id: I4c5c6abe08a135468d9ac25d44d6ea5b474131af
parent 1ebcbc67

cmds/idmap2/idmap2/CommandUtils.cpp

+14 −23
Original line number Diff line number Diff line
@@ -16,20 +16,15 @@ 


#include "idmap2/CommandUtils.h"



#include <android_content_res.h>



#include <fstream>

#include <memory>

#include <string>

#include <vector>



#include "androidfw/misc.h"

#include "idmap2/Idmap.h"

#include "idmap2/Result.h"

#include "idmap2/SysTrace.h"



using android::getFileModDate;

using android::toTimeT;

using android::idmap2::Error;

using android::idmap2::IdmapHeader;

using android::idmap2::OverlayResourceContainer;
@@ -47,25 +42,21 @@ Result<Unit> Verify(const std::string& idmap_path, const std::string& target_pat 
  if (!header) {

    return Error("failed to parse idmap header");

  }

  std::optional<Result<Unit>> header_ok;

  if (android_content_res_idmap_crc_is_mtime()) {

    header_ok = header->IsUpToDate(

        target_path, overlay_path, overlay_name, toTimeT(getFileModDate(target_path.c_str())),

        toTimeT(getFileModDate(overlay_path.c_str())), fulfilled_policies, enforce_overlayable);

  } else {



  auto target = TargetResourceContainer::FromPath(target_path);

  if (!target) {

    return Error("failed to load target '%s'", target_path.c_str());

  }



  auto overlay = OverlayResourceContainer::FromPath(overlay_path);

  if (!overlay) {

    return Error("failed to load overlay '%s'", overlay_path.c_str());

  }

    header_ok = header->IsUpToDate(**target, **overlay, overlay_name, fulfilled_policies,



  const auto header_ok = header->IsUpToDate(**target, **overlay, overlay_name, fulfilled_policies,

                                            enforce_overlayable);

  }

  if (!*header_ok) {

    return Error(header_ok->GetError(), "idmap not up to date");

  if (!header_ok) {

    return Error(header_ok.GetError(), "idmap not up to date");

  }

  return Unit{};

}

cmds/idmap2/idmap2d/Idmap2Service.cpp

+20 −29
Original line number Diff line number Diff line
@@ -16,8 +16,6 @@ 


#include "idmap2d/Idmap2Service.h"



#include <android_content_res.h>

#include <fcntl.h>

#include <sys/stat.h>   // umask

#include <sys/types.h>  // umask
@@ -41,6 +39,7 @@ 
#include "idmap2/PrettyPrintVisitor.h"

#include "idmap2/Result.h"

#include "idmap2/SysTrace.h"

#include <fcntl.h>



using android::base::StringPrintf;

using android::binder::Status;
@@ -52,9 +51,7 @@ using android::idmap2::IdmapConstraints; 
using android::idmap2::IdmapHeader;

using android::idmap2::OverlayResourceContainer;

using android::idmap2::PrettyPrintVisitor;

using android::idmap2::Result;

using android::idmap2::TargetResourceContainer;

using android::idmap2::Unit;

using android::idmap2::utils::kIdmapCacheDir;

using android::idmap2::utils::kIdmapFilePermissionMask;

using android::idmap2::utils::RandomStringForPath;
@@ -152,37 +149,31 @@ Status Idmap2Service::verifyIdmap(const std::string& target_path, const std::str 
    return ok();

  }



  std::optional<Result<Unit>> up_to_date;

  if (android_content_res_idmap_crc_is_mtime()) {

    up_to_date = header->IsUpToDate(

        target_path, overlay_path, overlay_name, toTimeT(getFileModDate(target_path.c_str())),

        toTimeT(getFileModDate(overlay_path.c_str())),

        ConvertAidlArgToPolicyBitmask(fulfilled_policies), enforce_overlayable);

  } else {

  const auto target = GetTargetContainer(target_path);

  if (!target) {

    *_aidl_return = false;

    LOG(WARNING) << "failed to load target '" << target_path << "'";

    return ok();

  }



  const auto overlay = OverlayResourceContainer::FromPath(overlay_path);

  if (!overlay) {

    *_aidl_return = false;

    LOG(WARNING) << "failed to load overlay '" << overlay_path << "'";

    return ok();

  }

    up_to_date =



  auto up_to_date =

      header->IsUpToDate(*GetPointer(*target), **overlay, overlay_name,

                         ConvertAidlArgToPolicyBitmask(fulfilled_policies), enforce_overlayable);

  }



  std::unique_ptr<const IdmapConstraints> newConstraints =

          ConvertAidlConstraintsToIdmapConstraints(constraints);



  *_aidl_return = static_cast<bool>(*up_to_date && (*oldConstraints == *newConstraints));

  if (!*up_to_date) {

  *_aidl_return = static_cast<bool>(up_to_date && (*oldConstraints == *newConstraints));

  if (!up_to_date) {

    LOG(WARNING) << "idmap '" << idmap_path

                 << "' not up to date : " << up_to_date->GetErrorMessage();

                 << "' not up to date : " << up_to_date.GetErrorMessage();

  }

  return ok();

}

cmds/idmap2/include/idmap2/Idmap.h

+0 −7
Original line number Diff line number Diff line
@@ -113,13 +113,6 @@ class IdmapHeader { 
    return version_;

  }



  // NOTE: The CRC fields used to be literal crc32, but now are just a way to identify if the

  // corresponding file has changed, so it's a stat.st_mtime now.

  // This means we may get false positives when the file changes, but the resources inside stay

  // the same. But it is so much faster to get and verify (a single stat() call instead of fully

  // parsing a zip archive and calculating a crc of the resources inside), that it is worth it:

  // false positives just make us re-create the idmaps occasionally and cause no correctness bugs.



  inline uint32_t GetTargetCrc() const {

    return target_crc_;

  }

cmds/idmap2/libidmap2/FabricatedOverlay.cpp

+5 −12
Original line number Diff line number Diff line
@@ -16,23 +16,22 @@ 


#include "idmap2/FabricatedOverlay.h"



#include <sys/stat.h>   // umask

#include <sys/types.h>  // umask



#include <android-base/file.h>

#include <android-base/strings.h>

#include <android_content_res.h>

#include <androidfw/BigBuffer.h>

#include <androidfw/BigBufferStream.h>

#include <androidfw/FileStream.h>

#include <androidfw/Image.h>

#include <androidfw/Png.h>

#include <androidfw/ResourceUtils.h>

#include <androidfw/Streams.h>

#include <androidfw/StringPiece.h>

#include <androidfw/StringPool.h>

#include <androidfw/Streams.h>

#include <google/protobuf/io/coded_stream.h>

#include <google/protobuf/io/zero_copy_stream_impl.h>

#include <sys/stat.h>   // umask

#include <sys/types.h>  // umask

#include <sys/utsname.h>

#include <utils/ByteOrder.h>

#include <zlib.h>
@@ -41,6 +40,7 @@ 
#include <memory>

#include <string>

#include <utility>

#include <sys/utsname.h>



namespace android::idmap2 {

constexpr auto kBufferSize = 1024;
@@ -486,13 +486,6 @@ Result<OverlayData> FabContainer::GetOverlayData(const OverlayManifestInfo& info 
}



Result<uint32_t> FabContainer::GetCrc() const {

  if (android_content_res_idmap_crc_is_mtime()) {

    auto mod_time = getFileModDate(path_.c_str());

    if (mod_time == kInvalidModDate) {

      return Error("Failed to get the modification time");

    }

    return toTimeT(mod_time);

  }

  return overlay_.GetCrc();

}

cmds/idmap2/libidmap2/ResourceContainer.cpp

+1 −9
Original line number Diff line number Diff line
@@ -16,8 +16,6 @@ 


#include "idmap2/ResourceContainer.h"



#include <android_content_res.h>



#include <memory>

#include <mutex>

#include <string>
@@ -59,13 +57,6 @@ const LoadedPackage* GetPackageAtIndex0(const LoadedArsc* loaded_arsc) { 
}



Result<uint32_t> CalculateCrc(const ZipAssetsProvider* zip_assets) {

  if (android_content_res_idmap_crc_is_mtime()) {

    auto mod_date = zip_assets->GetModDate();

    if (mod_date == kInvalidModDate) {

      return Error("failed to get modification date");

    }

    return toTimeT(mod_date);

  }

  constexpr const char* kResourcesArsc = "resources.arsc";

  std::optional<uint32_t> res_crc = zip_assets->GetCrc(kResourcesArsc);

  if (!res_crc) {
@@ -77,6 +68,7 @@ Result<uint32_t> CalculateCrc(const ZipAssetsProvider* zip_assets) { 
  if (!man_crc) {

    return Error("failed to get CRC for '%s'", kManifest);

  }



  return *res_crc ^ *man_crc;

}