Skip to content

Commit 35f5a01

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[har] Correctly preserve port information.
This aligns our HAR import/export with Firefox, and stores the remote port into the "connection" field of the HAR entry, and the Chrome specific connectionId into a custom "_connectionId" field. The spec[^1] doesn't strictly require this, but hints that the "connection" field is a good place to store the (server) port, and since Firefox already does it this way, we can just as well follow suit. [^1]: http://www.softwareishard.com/blog/har-12-spec/ Bug: 369859833 Change-Id: Ib2cfe8e30da745ce91e88fbc5768b68cef2e5403 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5938877 Reviewed-by: Philip Pfaffe <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]> Auto-Submit: Benedikt Meurer <[email protected]>
1 parent 6605054 commit 35f5a01

File tree

5 files changed

+48
-8
lines changed

5 files changed

+48
-8
lines changed

front_end/models/har/HARFormat.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ export class HAREntry extends HARBase {
168168
this.comment = HARBase.optionalString(data['comment']);
169169

170170
// Chrome specific.
171+
this.custom.set('connectionId', HARBase.optionalString(data['_connectionId']));
171172
this.custom.set('fromCache', HARBase.optionalString(data['_fromCache']));
172173
this.custom.set('initiator', this.importInitiator(data['_initiator']));
173174
this.custom.set('priority', HARBase.optionalString(data['_priority']));

front_end/models/har/Importer.test.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const exampleLog = new HAR.HARFormat.HARLog({
2525
}],
2626
entries: [
2727
{
28+
_connectionId: '1',
2829
_initiator: {
2930
type: 'script',
3031
requestId: '12',
@@ -59,7 +60,7 @@ const exampleLog = new HAR.HARFormat.HARLog({
5960
_priority: 'High',
6061
_resourceType: 'xhr',
6162
cache: {},
62-
connection: '1',
63+
connection: '6789',
6364
request: {
6465
method: 'POST',
6566
url: 'https://example.com/api/testEndpoint?param1=test',
@@ -118,6 +119,7 @@ const exampleLog = new HAR.HARFormat.HARLog({
118119
},
119120
{
120121
pageref: 'page_0',
122+
_connectionId: '1',
121123
_initiator: {
122124
type: 'script',
123125
stack: {
@@ -133,7 +135,7 @@ const exampleLog = new HAR.HARFormat.HARLog({
133135
},
134136
},
135137
cache: {},
136-
connection: '1',
138+
connection: '6789',
137139
request: {
138140
method: 'POST',
139141
url: 'https://example.com/api/testEndpoint?param2=test2',
@@ -286,4 +288,16 @@ describe('HAR Importer', () => {
286288
workerStart: 30,
287289
});
288290
});
291+
292+
it('Parses the remote address correctly', () => {
293+
for (const request of requests) {
294+
assert.strictEqual(request.remoteAddress(), '127.0.0.1:6789');
295+
}
296+
});
297+
298+
it('Parses the Chrome-specific connection ID', () => {
299+
for (const request of requests) {
300+
assert.strictEqual(request.connectionId, '1');
301+
}
302+
});
289303
});

front_end/models/har/Importer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class Importer {
7070
} else {
7171
request.setRequestFormData(false, null);
7272
}
73-
request.connectionId = entry.connection || '';
73+
request.connectionId = entry.customAsString('connectionId') || '';
7474
request.requestMethod = entry.request.method;
7575
request.setRequestHeaders(entry.request.headers);
7676

@@ -121,7 +121,7 @@ export class Importer {
121121
Importer.setupTiming(request, issueTime, entry.time, entry.timings);
122122

123123
// Meta data.
124-
request.setRemoteAddress(entry.serverIPAddress || '', 80); // Har does not support port numbers.
124+
request.setRemoteAddress(entry.serverIPAddress || '', Number(entry.connection) || 80);
125125
request.setResourceType(Importer.getResourceType(request, entry, pageLoad));
126126

127127
const priority = entry.customAsString('priority');

front_end/models/har/Log.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,29 @@ describe('HAR', () => {
116116
assert.strictEqual(entry._initiator?.requestId, requestId);
117117
});
118118

119+
it('exports remote address', async () => {
120+
const request = SDK.NetworkRequest.NetworkRequest.create(
121+
requestId, url, Platform.DevToolsPath.EmptyUrlString, null, null,
122+
{requestId, type: Protocol.Network.InitiatorType.Script});
123+
request.setRemoteAddress('127.0.0.1', 6789);
124+
125+
const entry = await build(request, {sanitize: false});
126+
127+
assert.strictEqual(entry.serverIPAddress, '127.0.0.1');
128+
assert.strictEqual(entry.connection, '6789');
129+
});
130+
131+
it('exports Chrome-specific connection ID', async () => {
132+
const request = SDK.NetworkRequest.NetworkRequest.create(
133+
requestId, url, Platform.DevToolsPath.EmptyUrlString, null, null,
134+
{requestId, type: Protocol.Network.InitiatorType.Script});
135+
request.connectionId = 'foobar';
136+
137+
const entry = await build(request, {sanitize: false});
138+
139+
assert.strictEqual(entry._connectionId, 'foobar');
140+
});
141+
119142
it('exports Service Worker info', async () => {
120143
const request = SDK.NetworkRequest.NetworkRequest.create(
121144
requestId, url, Platform.DevToolsPath.EmptyUrlString, null, null,

front_end/models/har/Log.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,10 @@ export class Entry {
114114
const harEntry = new Entry(request);
115115
let ipAddress = harEntry.request.remoteAddress();
116116
const portPositionInString = ipAddress.lastIndexOf(':');
117+
const connection = portPositionInString !== -1 ? ipAddress.substring(portPositionInString + 1) : undefined;
117118
if (portPositionInString !== -1) {
118119
ipAddress = ipAddress.substr(0, portPositionInString);
119120
}
120-
121121
const timings = harEntry.buildTimings();
122122
let time = 0;
123123
// "ssl" is included in the connect field, so do not double count it.
@@ -146,13 +146,14 @@ export class Entry {
146146
}
147147

148148
const entry: EntryDTO = {
149+
_connectionId: undefined,
149150
_fromCache: undefined,
150151
_initiator: exportedInitiator,
151152
_priority: harEntry.request.priority(),
152153
_resourceType: harEntry.request.resourceType().name(),
153154
_webSocketMessages: undefined,
154155
cache: {},
155-
connection: undefined,
156+
connection,
156157
pageref: undefined,
157158
request: await harEntry.buildRequest(),
158159
response: harEntry.buildResponse(),
@@ -183,9 +184,9 @@ export class Entry {
183184
}
184185

185186
if (harEntry.request.connectionId !== '0') {
186-
entry.connection = harEntry.request.connectionId;
187+
entry._connectionId = harEntry.request.connectionId;
187188
} else {
188-
delete entry.connection;
189+
delete entry._connectionId;
189190
}
190191

191192
const page = SDK.PageLoad.PageLoad.forRequest(harEntry.request);
@@ -507,6 +508,7 @@ export interface Response {
507508
}
508509

509510
export interface EntryDTO {
511+
_connectionId?: string;
510512
_fromCache?: string;
511513
_initiator: Protocol.Network.Initiator|null;
512514
_priority: Protocol.Network.ResourcePriority|null;

0 commit comments

Comments
 (0)