CefStringBase::DetachToUserFree modifies data it does not own

[zhouzuoji]

‌

  userfree_struct_type DetachToUserFree() {
    if (empty())
      return NULL;

    userfree_struct_type str = traits::userfree_alloc();
    memcpy(str, string_, sizeof(struct_type));

    // Free this class' structure but not the data.
    memset(string_, 0, sizeof(struct_type));
    ClearAndFree();

    return str;
  }

memset(string_, 0, sizeof(struct_type));

This line fill zeroes tostring_, which it may not own.

this is the reason why CefRequestContext::GetCachePath() behaves correctly only at first call, and return empty string at later calls.

‌

[Marshall Greenblatt]

I believe the problem is this:

CefString CefRequestContextImpl::GetCachePath() {
  return CefString(&config_.settings.cache_path);
}

Which uses this CefString constructor:

  ///
  // Create a new string referencing an existing string structure without taking
  // ownership. Referenced structures must exist for the lifetime of this class
  // and will not be freed by this class.
  ///
  CefStringBase(const struct_type* src) : string_(NULL), owner_(false) {
    if (!src)
      return;
    // Reference the existing structure without taking ownership.
    Attach(const_cast<struct_type*>(src), false);
  }

So the CefString returned from GetCachePath references config_.settings.cache_path instead of making a copy. Then, the call to DetachToUserFree unintentionally clears config_.settings.cache_path.

[Marshall Greenblatt]

I think the correct (general) solution is for DetachToUserFree to copy strings instead of transferring ownership when owner_==false.

[Marshall Greenblatt]

Fix unintentional state transfer in DetachToUserFree (fixes issue #3309)

Calling DetachToUserFree() on a CefString holding a reference should copy the value instead of transferring ownership.

A new StringTest.Ownership test has been added for this behavior.

→ <<cset 4921dc22135e>>

[Marshall Greenblatt]

Fix unintentional state transfer in DetachToUserFree (fixes issue #3309)

Calling DetachToUserFree() on a CefString holding a reference should copy the value instead of transferring ownership.

A new StringTest.Ownership test has been added for this behavior.

→ <<cset e56440898e62>>

[Marshall Greenblatt]

Fix unintentional state transfer in DetachToUserFree (fixes issue #3309)

Calling DetachToUserFree() on a CefString holding a reference should copy the value instead of transferring ownership.

A new StringTest.Ownership test has been added for this behavior.

→ <<cset c36c371f68c0>>

[zhouzuoji]

Thank you for fixing the issue so quickly.