Skip to content

Commit 6dec4b0

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Only render images with data url from resource model
The `ImageRef` component was rendering URLs from the trace, which is a potential security issue given the devtools page has access to CDP. Instead, only render images if found in the active resource tree model, and use the data url given by `Page.getResourceContent`. See: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6532244/comment/c8aa4c3c_edff91ce/ Bug: 416449602 Change-Id: I3bc3c6f48456a13dc7dd10d0dfb66f3570c3c406 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6605300 Commit-Queue: Connor Clark <cjamcl@chromium.org> Reviewed-by: Paul Irish <paulirish@chromium.org>
1 parent e106eff commit 6dec4b0

File tree

2 files changed

+40
-14
lines changed

2 files changed

+40
-14
lines changed

front_end/models/trace/insights/DocumentLatency.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ export const UIStrings = {
4242
* @description Text to tell the user that the time starting the document request to when the server started responding is acceptable.
4343
* @example {600 ms} PH1
4444
*/
45-
passingServerResponseTime: 'Server responds quickly (observed {PH1}) ',
45+
passingServerResponseTime: 'Server responds quickly (observed {PH1})',
4646
/**
4747
* @description Text to tell the user that the time starting the document request to when the server started responding is not acceptable.
4848
* @example {601 ms} PH1
4949
*/
50-
failedServerResponseTime: 'Server responded slowly (observed {PH1}) ',
50+
failedServerResponseTime: 'Server responded slowly (observed {PH1})',
5151
/**
5252
* @description Text to tell the user that text compression (like gzip) was applied.
5353
*/

front_end/panels/timeline/components/insights/EventRef.ts

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import * as i18n from '../../../../core/i18n/i18n.js';
77
import * as Platform from '../../../../core/platform/platform.js';
8+
import * as SDK from '../../../../core/sdk/sdk.js';
89
import * as Trace from '../../../../models/trace/trace.js';
910
import * as ComponentHelpers from '../../../../ui/components/helpers/helpers.js';
1011
import * as Lit from '../../../../ui/lit/lit.js';
@@ -81,27 +82,57 @@ class ImageRef extends HTMLElement {
8182
readonly #shadow = this.attachShadow({mode: 'open'});
8283

8384
#request?: Trace.Types.Events.SyntheticNetworkRequest;
85+
#imageDataUrl?: string|null;
8486

8587
set request(request: Trace.Types.Events.SyntheticNetworkRequest) {
8688
this.#request = request;
89+
this.#imageDataUrl = undefined;
8790
void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
8891
}
8992

90-
#render(): void {
93+
/**
94+
* This only returns a data url if the resource is currently present from the active
95+
* inspected page.
96+
*/
97+
async #getOrCreateImageDataUrl(): Promise<string|null> {
98+
if (!this.#request) {
99+
return null;
100+
}
101+
102+
if (this.#imageDataUrl !== undefined) {
103+
return this.#imageDataUrl;
104+
}
105+
106+
const originalUrl = this.#request.args.data.url as Platform.DevToolsPath.UrlString;
107+
const resource = SDK.ResourceTreeModel.ResourceTreeModel.resourceForURL(originalUrl);
108+
if (!resource) {
109+
this.#imageDataUrl = null;
110+
return this.#imageDataUrl;
111+
}
112+
113+
const content = await resource.requestContentData();
114+
if ('error' in content) {
115+
this.#imageDataUrl = null;
116+
return this.#imageDataUrl;
117+
}
118+
119+
this.#imageDataUrl = content.asDataUrl();
120+
return this.#imageDataUrl;
121+
}
122+
123+
async #render(): Promise<void> {
91124
if (!this.#request) {
92125
return;
93126
}
94127

128+
const url = this.#request.args.data.mimeType.includes('image') ? await this.#getOrCreateImageDataUrl() : null;
129+
const img = url ? html`<img src=${url} class="element-img"/>` : Lit.nothing;
130+
95131
// clang-format off
96132
Lit.render(html`
97133
<style>${baseInsightComponentStyles}</style>
98134
<div class="image-ref">
99-
${this.#request.args.data.mimeType.includes('image') ? html`
100-
<img
101-
class="element-img"
102-
src=${this.#request.args.data.url}
103-
@error=${handleBadImage}/>
104-
`: Lit.nothing}
135+
${img}
105136
<span class="element-img-details">
106137
${eventRef(this.#request)}
107138
<span class="element-img-details-size">${
@@ -114,11 +145,6 @@ class ImageRef extends HTMLElement {
114145
}
115146
}
116147

117-
function handleBadImage(event: Event): void {
118-
const img = event.target as HTMLImageElement;
119-
img.style.display = 'none';
120-
}
121-
122148
export function imageRef(request: Trace.Types.Events.SyntheticNetworkRequest): Lit.TemplateResult {
123149
return html`
124150
<devtools-performance-image-ref

0 commit comments

Comments
 (0)