[Enterprise] Add WebSocket support for HTTP header injection [chromium/src : main]

0 views

Skip to first unread message

Anatoli Hancharou (Gerrit)

unread,

1:45 PM (8 hours ago) 1:45 PM

to android-bu...@system.gserviceaccount.com, Dave Tapuska, Sebastien Lalancette, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Dave Tapuska

Anatoli Hancharou added 1 comment

File content/browser/websockets/websocket_connector_impl.cc

Line 144, Patchset 8 (Parent): mojo::NullRemote(), std::move(throttling_profile_id),

Dave Tapuska . resolved

Since you really want to influence this NullRemote being passed in here. I really suggest changing ContentBrowserClient::GetWebSocketOptions to return the trusted header mojo remote along side with the options value. That would be much cleaner.
You'd likely need to some some magic inside the extension intercepting the WebSocket anyways, but I'm not sure you want to provide the header in that case or not.

Anatoli Hancharou

Great catch!

I refactored the code to use header client wrapper that allows injecting 'HeaderInjectionClient' into extensions interceptor and registering it after `WebRequestProxyingWebSocket` to ensure precedence

Open in Gerrit

Related details

Attention is currently required from:

Dave Tapuska

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Dave Tapuska (Gerrit)

unread,

4:00 PM (5 hours ago) 4:00 PM

to Anatoli Hancharou, android-bu...@system.gserviceaccount.com, Sebastien Lalancette, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Anatoli Hancharou

Dave Tapuska added 3 comments

File chrome/browser/chrome_content_browser_client.cc

Line 6876, Patchset 11 (Latest): std::move(handshake_client), std::move(header_client_wrapper));

Dave Tapuska . unresolved

If WebSocketOptions is passed in here, you very well can define your own base::OnceCallback inside Extensions (as opposed to the content layer) that lets you create you intermediate trusted header injector...

File content/public/browser/content_browser_client.h

Line 2181, Patchset 11 (Latest): WebSocketHeaderClientWrapper header_client_wrapper);

Dave Tapuska . unresolved

Can we pass WebSocketOptions here instead?

Line 2151, Patchset 11 (Latest): WebSocketHeaderClientWrapper header_client_wrapper;

Dave Tapuska . unresolved

I'd prefer if this wasn't a callback, just allocate the PendingRemove here as a member.

Open in Gerrit

Related details

Attention is currently required from:

Anatoli Hancharou

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages