Bug 311707
| Summary: | CredentialsContainer: performCommonChecks() should return RefPtr<Document> instead of bool | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Marcos Caceres <marcosc> |
| Component: | WebKit Misc. | Assignee: | Marcos Caceres <marcosc> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Local Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Marcos Caceres
In CredentialsContainer.cpp, `get()` and `isCreate()` both call `performCommonChecks()` which checks for a null document internally via RefPtr, then immediately re-acquire the document via `Ref document = *this->document()` — relying on an implicit contract that the document is still non-null.
```cpp
void CredentialsContainer::get(CredentialRequestOptions&& options, CredentialPromise&& promise)
{
if (!performCommonChecks(options, promise))
return;
Ref document = *this->document(); // unsafe: re-acquires WeakPtr without null check
...
}
```
The fix is to change `performCommonChecks()` to return `RefPtr<Document>` (or null on failure), so callers receive the document directly without re-acquiring the WeakPtr:
```cpp
void CredentialsContainer::get(CredentialRequestOptions&& options, CredentialPromise&& promise)
{
RefPtr document = performCommonChecks(options, promise);
if (!document)
return;
...
}
```
This makes the null-safety contract explicit and eliminates the implicit assumption.
Note: This is pre-existing code. In practice the implicit contract holds soundly under WebKit's single-threaded execution model, but it violates the spirit of WebKit's safer-cpp guidelines.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/174293959>
Marcos Caceres
Pull request: https://github.com/WebKit/WebKit/pull/62256
EWS
Committed 310779@main (acecc70c2382): <https://commits.webkit.org/310779@main>
Reviewed commits have been landed. Closing PR #62256 and removing active labels.