From 5411abaa784a0d8a4731fd00d9de61772a0205ab Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 14 Feb 2020 00:11:59 -0500 Subject: [PATCH 01/18] Bridge: specify methods for info re: powerbox requests/offers. The next few commits will flesh out relevant documentation to mention the header described in the comments, and describe how to use these methods. All this will serve as a more precise spec for what to implement. --- src/sandstorm/sandstorm-http-bridge.capnp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/sandstorm/sandstorm-http-bridge.capnp b/src/sandstorm/sandstorm-http-bridge.capnp index d01a2024f1..d78176ac60 100644 --- a/src/sandstorm/sandstorm-http-bridge.capnp +++ b/src/sandstorm/sandstorm-http-bridge.capnp @@ -21,6 +21,7 @@ $import "/capnp/c++.capnp".namespace("sandstorm"); using Grain = import "grain.capnp"; using Identity = import "identity.capnp"; +using Powerbox = import "powerbox.capnp"; interface SandstormHttpBridge { # Bootstrap interface provided to the app on a Unix domain socket at /tmp/sandstorm-api. @@ -32,6 +33,16 @@ interface SandstormHttpBridge { # Get the SessionContext corresponding to a UiSession. The appropriate `id` value can be read # from the X-Sandstorm-Session-Id header inserted by sandstorm-http-bridge. + getSessionRequest @4 (id :Text) -> (requestInfo :List(Powerbox.PowerboxDescriptor)); + # Get the requestInfo for a powerbox request session. The `id` parameter is the same as for + # `getSessionContext`. This is only valid if the current session actually is a request + # session. If it is, the `X-Sandstorm-Session-Type` header will be set to 'request'. + + getSessionOffer @5 (id :Text) -> (offer :Capability, descriptor :Powerbox.PowerboxDescriptor); + # Get the offer information for a powerbox offer session. The `id` parameter is the same as + # for `getSessionContext`. This is only valid if the current session actually is an offer + # session. If it is, the `X-Sandstorm-Session-Type` header will be set to 'offer'. + getSavedIdentity @2 (identityId :Text) -> (identity :Identity.Identity); # If BridgeConfig.saveIdentityCaps is true for this app, then you can call this method to fetch # the saved identity capability for a particular identityId as passed in the From e234257e04a872e993e0f4cd42cfc1baf202e645 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 14 Feb 2020 18:34:12 -0500 Subject: [PATCH 02/18] Update docs to describe using non-http apis via the bridge. Still need to talk about offer sessions. --- docs/developing/powerbox.md | 55 +++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/docs/developing/powerbox.md b/docs/developing/powerbox.md index 3c9d5043f9..728d27da08 100644 --- a/docs/developing/powerbox.md +++ b/docs/developing/powerbox.md @@ -167,16 +167,20 @@ to your app's server, e.g. using `XmlHTTPRequest`. ## Redeeming the claim token -Once the claim token has been sent to your app's server, you need to redeem it. How to do that -depends on whether your app uses sandstorm-http-bridge. +Once the claim token has been sent to your app's server, you need to redeem it. In the general +case, you can use the raw Cap'n Proto APIs to do this, but sandstorm-http-bridge implements +special support for using http APIs obtained via the powerbox. -### Using sandstorm-http-bridge +### Using sandstorm-http-bridge for HTTP APIs. Most apps use sandstorm-http-bridge to avoid the need to use Sandstorm's raw Cap'n Proto -interfaces. If you aren't sure whether you are using http-bridge, you probably are. +interfaces just to offer a web interface. If you aren't sure whether you are using http-bridge, +you probably are. + +If you want to use the powerbox to access something *other than an HTTP API*, you will need to +use the raw Cap'n Proto APIs, see below. -Currently, **http-bridge apps can only request and use HTTP APIs**, not arbitrary Cap'n Proto -APIs. HTTP APIs are represented by the `ApiSession` Cap'n Proto type. (The query examples above +HTTP APIs are represented by the `ApiSession` Cap'n Proto type. (The query examples above request this type.) When using http-bridge, the bridge sets up a private HTTP proxy which your app can use to make @@ -261,11 +265,14 @@ matters. ### Raw Cap'n Proto APIs -If your app uses raw Cap'n Proto APIs (not http-bridge), then the definitive reference for the -powerbox's interfaces is the Cap'n Proto schema files where they are defined. The main relevant -schemas are +If your app uses raw Cap'n Proto APIs (not http-bridge), or you want to use something other than +an HTTP api, then the definitive reference for the powerbox's interfaces is the Cap'n Proto +schema files where they are defined. The main relevant schemas are [powerbox.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/powerbox.capnp) and [grain.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/grain.capnp). +If you are using the bridge, you will also want to read and +[sandstorm-http-bridge.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/sandstorm-http-bridge.capnp), which allows bridge apps to access the resources otherwise accessed via +the interfaces in `grain.capnp`. In order to exchange your claim token for a capability, you'll need to invoke `SessionContext.claimRequest()` on the session context associated with the session where the @@ -276,20 +283,22 @@ this capability later, e.g. during a future run of your app. You can't just hold token, because the claim token can only be redeemed against the specific session from which it came. `save()` gives you a token that can be redeemed using `SandstormApi.restore()` at any time. -## Exporting an HTTP API +## Exporting an API Your app can also export APIs for consumption by other apps. If you do so, then grains of your app will appear as options in the user's powerbox when another app makes a request for an API that your app provides. -How to export APIs depends on whether your app uses sandstorm-http-bridge. +How to export APIs depends on (1) whether your app uses sandstorm-http-bridge and (2) +whether you are exporting an HTTP API or some other Cap'n Proto interface. ### Using sandstorm-http-bridge -If you use sandstorm-http-bridge, then you can only export HTTP APIs, which implement the -Cap'n Proto `ApiSession` interface. You can declare APIs that you export in your -`sandstorm-pkgdef.capnp` file, in the `bridgeConfig` section, by specifying a list of -`powerboxApis`. Example: +If you use sandstorm-http-bridge, then you can export HTTP APIs (which implement +the Cap'n Proto `ApiSession`) using the bridge's, special handling, allowing you to use +a regular web server just like with the web UI for your app. You can declare APIs that +you export in your `sandstorm-pkgdef.capnp` file, in the `bridgeConfig` section, by +specifying a list of `powerboxApis`. Example: ```capnp bridgeConfig = ( @@ -332,6 +341,22 @@ permissions levels of the same API. If a powerbox request matches multiple APIs, chooses a grain of your app to satisfy the request, then they will be presented with a choice of which API to use, with options labeled using `displayInfo`. +It is also possible to export Cap'n Proto interfaces other than HTTP APIs when +using the bridge. The process is the same as discussed for "Raw Cap'n Proto APIs," below, except +that: + +1. You need to use the interfaces in `sandstorm-http-bridge.capnp` to access the session context, + provide the view info for your app, etc. +2. Rather than implementing separate methods for `newSession`, `newRequestSession`, and so on, + you should look at the `X-Sandstorm-Session-Type` header to determine what kind of session + you are in. + - If the header's value is `normal`, then you are not in any kind of powerbox session; this + is just a regular UI session. + - If the header's value is `request`, then you are in a request session. You should display + a UI for picking which resource to provide, and use the methods described in + `sandstorm-http-bridge.capnp` to fetch info about the powerbox request and fulfill it. + - If the value is `offer`, this is an offer session. TODO: talk more about offer sessions. + ### Raw Cap'n Proto APIs When implementing an app against raw Cap'n Proto APIs, you have much more freedom. Not only can you From 9b11ed80c1bfabc79e54fdcff0b34a9e261c9663 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 14 Feb 2020 18:38:06 -0500 Subject: [PATCH 03/18] Add a little bit of info about offer sessions. --- docs/developing/powerbox.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/developing/powerbox.md b/docs/developing/powerbox.md index 728d27da08..0b99846efd 100644 --- a/docs/developing/powerbox.md +++ b/docs/developing/powerbox.md @@ -355,7 +355,10 @@ that: - If the header's value is `request`, then you are in a request session. You should display a UI for picking which resource to provide, and use the methods described in `sandstorm-http-bridge.capnp` to fetch info about the powerbox request and fulfill it. - - If the value is `offer`, this is an offer session. TODO: talk more about offer sessions. + - If the value is `offer`, this is an offer session; your app is being offered a capability by + another app, based on the information in your view info's `matchOffers` field. You can + display a UI to decide what to do with it, and use the bridge's methods to access the + capability itself. ### Raw Cap'n Proto APIs From 786e95868cf6548c91bed0f8f8492b9cdc6549dd Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 14 Feb 2020 19:33:30 -0500 Subject: [PATCH 04/18] Include X-Sandstorm-Session-Type: normal in existing sessions. Not tested (though it doesn't break the existing tests). --- .../sandstorm-http-bridge-internal.capnp | 6 +++++ src/sandstorm/sandstorm-http-bridge.c++ | 23 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge-internal.capnp b/src/sandstorm/sandstorm-http-bridge-internal.capnp index 34c17972b3..64885d5946 100644 --- a/src/sandstorm/sandstorm-http-bridge-internal.capnp +++ b/src/sandstorm/sandstorm-http-bridge-internal.capnp @@ -58,3 +58,9 @@ struct BridgeObjectId { interface BridgeHttpSession extends(ApiSession, AppPersistent(BridgeObjectId)) {} const bridgeRequestSessionHtml :Text = embed "sandstorm-http-bridge-request.html"; + +enum SessionType { + normal @0; + request @1; + offer @2; +} diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 6529103cc8..a41385ee6b 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1353,7 +1353,8 @@ public: kj::String&& basePath, kj::String&& userAgent, kj::String&& acceptLanguages, kj::String&& rootPath, kj::String&& permissions, kj::Maybe remoteAddress, - kj::Maybe>&& apiInfo) + kj::Maybe>&& apiInfo, + SessionType sessionType) : serverAddr(serverAddr), sessionContext(kj::mv(sessionContext)), bridgeContext(bridgeContext), @@ -1369,7 +1370,8 @@ public: acceptLanguages(kj::mv(acceptLanguages)), rootPath(kj::mv(rootPath)), remoteAddress(kj::mv(remoteAddress)), - apiInfo(kj::mv(apiInfo)) { + apiInfo(kj::mv(apiInfo)), + sessionType(sessionType) { if (userInfo.hasIdentityId()) { userId = textIdentityId(userInfo.getIdentityId()); } @@ -1636,6 +1638,7 @@ private: spk::BridgeConfig::Reader config; kj::Maybe remoteAddress; kj::Maybe> apiInfo; + SessionType sessionType; kj::String makeHeaders(kj::StringPtr method, kj::StringPtr path, WebSession::Context::Reader context, @@ -1697,6 +1700,13 @@ private: enumerants[pronounValue].getProto().getName())); } } + { + capnp::EnumSchema schema = capnp::Schema::from(); + uint typeValue = static_cast(sessionType); + auto enumerants = schema.getEnumerants(); + lines.add(kj::str("X-Sandstorm-Session-Type: ", + enumerants[typeValue].getProto().getName())); + } lines.add(kj::str("X-Sandstorm-Permissions: ", permissions)); if (basePath.size() > 0) { lines.add(kj::str("X-Sandstorm-Base-Path: ", basePath)); @@ -1937,7 +1947,8 @@ WebSession::Client newPowerboxApiSession( nullptr, nullptr, nullptr, kj::str(httpApi.getPath(), '/'), bridgeContext.formatPermissions(httpApi.getPermissions()), - nullptr, kj::mv(httpApi))); + nullptr, kj::mv(httpApi), + SessionType::NORMAL)); }); }); } @@ -2346,7 +2357,8 @@ public: kj::strArray(sessionParams.getAcceptableLanguages(), ","), kj::heapString("/"), bridgeContext.formatPermissions(userPermissions), - nullptr, nullptr); + nullptr, nullptr, + SessionType::NORMAL); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { @@ -2367,7 +2379,8 @@ public: kj::heapString(""), kj::heapString(""), kj::heapString(""), kj::heapString(config.getApiPath()), bridgeContext.formatPermissions(userPermissions), - kj::mv(addr), nullptr); + kj::mv(addr), nullptr, + SessionType::NORMAL); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { From 9f7663399f228b7dd98fafb5630fc18bfc74f929 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Sat, 15 Feb 2020 16:05:59 -0500 Subject: [PATCH 05/18] Fix some clerical errors Thanks to @ocdtrekkie for spotting these. --- docs/developing/powerbox.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/developing/powerbox.md b/docs/developing/powerbox.md index 0b99846efd..d6832e344b 100644 --- a/docs/developing/powerbox.md +++ b/docs/developing/powerbox.md @@ -270,9 +270,10 @@ an HTTP api, then the definitive reference for the powerbox's interfaces is the schema files where they are defined. The main relevant schemas are [powerbox.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/powerbox.capnp) and [grain.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/grain.capnp). -If you are using the bridge, you will also want to read and -[sandstorm-http-bridge.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/sandstorm-http-bridge.capnp), which allows bridge apps to access the resources otherwise accessed via -the interfaces in `grain.capnp`. +If you are using the bridge, you will also want to read about the interfaces in +[sandstorm-http-bridge.capnp](https://github.com/sandstorm-io/sandstorm/blob/master/src/sandstorm/sandstorm-http-bridge.capnp), +which allow bridge apps to access the resources otherwise accessed via the interfaces in +`grain.capnp`. In order to exchange your claim token for a capability, you'll need to invoke `SessionContext.claimRequest()` on the session context associated with the session where the @@ -295,7 +296,7 @@ whether you are exporting an HTTP API or some other Cap'n Proto interface. ### Using sandstorm-http-bridge If you use sandstorm-http-bridge, then you can export HTTP APIs (which implement -the Cap'n Proto `ApiSession`) using the bridge's, special handling, allowing you to use +the Cap'n Proto `ApiSession`) using the bridge's special handling, allowing you to use a regular web server just like with the web UI for your app. You can declare APIs that you export in your `sandstorm-pkgdef.capnp` file, in the `bridgeConfig` section, by specifying a list of `powerboxApis`. Example: From f4fee3d2573502a845db1d1819b70b19210f1fb6 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Sun, 16 Feb 2020 17:52:06 -0500 Subject: [PATCH 06/18] BridgeContext: make the sessions map private. We're going to start tweaking this to support offer & request sessions, so now is a good time to do this cleanup. --- src/sandstorm/sandstorm-http-bridge.c++ | 30 +++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index a41385ee6b..cb8cc8b682 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1230,14 +1230,28 @@ public: } } - std::map sessions; - // TODO(cleanup): Make this private with appropriate accessor methods. + kj::Maybe findSession(const kj::StringPtr& id) { + auto iter = sessions.find(id); + if(iter != sessions.end()) { + return nullptr; + } + return iter->second; + } + + void eraseSession(const kj::StringPtr& id) { + sessions.erase(id); + } + + void insertSession(const kj::StringPtr& id, SessionContext::Client& session) { + sessions.insert({kj::StringPtr(id), session}); + } private: SandstormApi::Client apiCap; spk::BridgeConfig::Reader config; kj::AutoCloseFd identitiesDir; kj::AutoCloseFd trashDir; + std::map sessions; struct IdentityRecord { IdentityRecord(const IdentityRecord& other) = delete; @@ -1376,13 +1390,13 @@ public: userId = textIdentityId(userInfo.getIdentityId()); } if (this->sessionId != nullptr) { - bridgeContext.sessions.insert({kj::StringPtr(this->sessionId), this->sessionContext}); + bridgeContext.insertSession(kj::StringPtr(this->sessionId), this->sessionContext); } } ~WebSessionImpl() noexcept(false) { if (this->sessionId != nullptr) { - bridgeContext.sessions.erase(kj::StringPtr(sessionId)); + bridgeContext.eraseSession(kj::StringPtr(sessionId)); } } @@ -2248,9 +2262,11 @@ public: kj::Promise getSessionContext(GetSessionContextContext context) override { auto id = context.getParams().getId(); - auto iter = bridgeContext.sessions.find(id); - KJ_ASSERT(iter != bridgeContext.sessions.end(), "Session ID not found", id); - context.getResults().setContext(iter->second); + KJ_IF_MAYBE(value, bridgeContext.findSession(id)) { + context.getResults().setContext(*value); + } else { + KJ_FAIL_ASSERT("Session ID not found", id); + } return kj::READY_NOW; } From 9cde25654a7f6877f3d94461e2c818abd79e1c27 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Mon, 17 Feb 2020 16:52:47 -0500 Subject: [PATCH 07/18] First pass at handling get{Request,Offer}Session. This needs further testing, but is now feature complete wrt. #3197 --- src/sandstorm/sandstorm-http-bridge.c++ | 195 +++++++++++++++++++++--- 1 file changed, 176 insertions(+), 19 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index cb8cc8b682..2dc412e9e7 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1118,6 +1118,15 @@ private: } }; +template +kj::Maybe findInMap(std::map& map, const kj::StringPtr& id) { + auto iter = map.find(id); + if(iter != map.end()) { + return nullptr; + } + return iter->second; +} + class BridgeContext: private kj::TaskSet::ErrorHandler { public: BridgeContext(SandstormApi::Client apiCap, spk::BridgeConfig::Reader config) @@ -1231,21 +1240,63 @@ public: } kj::Maybe findSession(const kj::StringPtr& id) { - auto iter = sessions.find(id); - if(iter != sessions.end()) { + return findInMap(sessions, id); + } + + kj::Maybe findOffer(const kj::StringPtr& id) { + KJ_IF_MAYBE(message, findInMap(offers, id)) { + SandstormHttpBridge::GetSessionOfferResults::Reader ret = + (*message)->getRoot(); + return ret; + } else { + return nullptr; + } + } + + kj::Maybe findRequest(const kj::StringPtr& id) { + KJ_IF_MAYBE(message, findInMap(requests, id)) { + SandstormHttpBridge::GetSessionRequestResults::Reader ret = + (*message)->getRoot(); + return ret; + } else { return nullptr; } - return iter->second; } void eraseSession(const kj::StringPtr& id) { sessions.erase(id); + offers.erase(id); + requests.erase(id); } void insertSession(const kj::StringPtr& id, SessionContext::Client& session) { sessions.insert({kj::StringPtr(id), session}); } + void insertRequest( + const kj::StringPtr& id, + capnp::List::Reader descriptor) { + + kj::Own message; + auto results = message->getRoot(); + results.setRequestInfo(descriptor); + + requests.insert({kj::StringPtr(id), kj::mv(message)}); + } + + void insertOffer( + const kj::StringPtr& id, + capnp::Capability::Client&& offer, + PowerboxDescriptor::Reader descriptor) { + + kj::Own message; + auto results = message->getRoot(); + results.setOffer(offer); + results.setDescriptor(descriptor); + + offers.insert({kj::StringPtr(id), kj::mv(message)}); + } + private: SandstormApi::Client apiCap; spk::BridgeConfig::Reader config; @@ -1253,6 +1304,13 @@ private: kj::AutoCloseFd trashDir; std::map sessions; + // Contains SandstormHttpBridge::GetSessionRequestResults. TODO(zenhack): figure + // out if there's a way to make this typed: + std::map> requests; + + // Contains SandstormHttpBridge::GetSessionOfferResults. + std::map> offers; + struct IdentityRecord { IdentityRecord(const IdentityRecord& other) = delete; IdentityRecord(IdentityRecord&& other) = default; @@ -2270,6 +2328,26 @@ public: return kj::READY_NOW; } + kj::Promise getSessionOffer(GetSessionOfferContext context) override { + auto id = context.getParams().getId(); + KJ_IF_MAYBE(value, bridgeContext.findOffer(id)) { + context.setResults(*value); + } else { + KJ_FAIL_ASSERT("Session ID not found; maybe this isn't an offer session?"); + } + return kj::READY_NOW; + } + + kj::Promise getSessionRequest(GetSessionRequestContext context) override { + auto id = context.getParams().getId(); + KJ_IF_MAYBE(value, bridgeContext.findRequest(id)) { + context.setResults(*value); + } else { + KJ_FAIL_ASSERT("Session ID not found; maybe this isn't a request session?"); + } + return kj::READY_NOW; + } + kj::Promise getSavedIdentity(GetSavedIdentityContext context) override { context.getResults().setIdentity( bridgeContext.loadIdentity(context.getParams().getIdentityId())); @@ -2346,6 +2424,29 @@ public: return kj::READY_NOW; } + UiSession::Client newUiSession( + UserInfo::Reader userInfo, + kj::String&& sessionId, + WebSession::Params::Reader sessionParams, + SessionContext::Client sessionCtx, + kj::ArrayPtr tabId, + SessionType sessionType) { + + auto userPermissions = userInfo.getPermissions(); + return + kj::heap(serverAddress, userInfo, sessionCtx, + bridgeContext, kj::mv(sessionId), + kj::encodeHex(tabId), + kj::heapString(sessionParams.getBasePath()), + kj::heapString(sessionParams.getUserAgent()), + kj::strArray(sessionParams.getAcceptableLanguages(), ","), + kj::heapString("/"), + bridgeContext.formatPermissions(userPermissions), + nullptr, nullptr, + sessionType); + + } + kj::Promise newSession(NewSessionContext context) override { auto params = context.getParams(); auto sessionType = params.getSessionType(); @@ -2365,16 +2466,13 @@ public: auto sessionParams = params.getSessionParams().getAs(); UiSession::Client session = - kj::heap(serverAddress, userInfo, params.getContext(), - bridgeContext, kj::str(sessionIdCounter++), - kj::encodeHex(params.getTabId()), - kj::heapString(sessionParams.getBasePath()), - kj::heapString(sessionParams.getUserAgent()), - kj::strArray(sessionParams.getAcceptableLanguages(), ","), - kj::heapString("/"), - bridgeContext.formatPermissions(userPermissions), - nullptr, nullptr, - SessionType::NORMAL); + newUiSession( + userInfo, + kj::str(sessionIdCounter++), + sessionParams, + params.getContext(), + params.getTabId(), + SessionType::NORMAL); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { @@ -2422,15 +2520,74 @@ public: auto permissions = kj::heapArrayFromIterable(userInfo.getPermissions()); + auto requestInfo = params.getRequestInfo(); + bool allApiSession = true; + for(const auto& desc : requestInfo) { + for(const auto& tag : desc.getTags()) { + if(tag.getId() != capnp::typeId()) { + allApiSession = false; + break; + } + } + } + + if(allApiSession) { + // All of the tags are of type ApiSession; handle the request ourselves. + UiSession::Client session = + kj::heap( + serverAddress, bridgeContext, params.getContext(), + kj::heapArray(userInfo.getIdentityId()), kj::mv(permissions)); + + context.getResults(capnp::MessageSize {2, 1}).setSession( + connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { + return kj::mv(session); + })); + } else { + // At least one tag is something other than ApiSession; let the app handle + // the request. + auto sessionId = kj::str(sessionIdCounter++); + + bridgeContext.insertRequest(kj::str(sessionId), requestInfo); + auto sessionParams = params.getSessionParams().getAs(); + UiSession::Client session = + newUiSession( + userInfo, + kj::str(sessionIdCounter++), + sessionParams, + params.getContext(), + params.getTabId(), + SessionType::REQUEST); + + context.getResults(capnp::MessageSize {2, 1}).setSession( + connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { + return kj::mv(session); + })); + } + return kj::READY_NOW; + } + + kj::Promise newOfferSession(NewOfferSessionContext context) override { + auto params = context.getParams(); + auto sessionId = kj::str(sessionIdCounter++); + auto userInfo = params.getUserInfo(); + auto sessionParams = params.getSessionParams().getAs(); + + bridgeContext.insertOffer(kj::str(sessionId), params.getOffer(), params.getDescriptor()); + UiSession::Client session = - kj::heap( - serverAddress, bridgeContext, params.getContext(), - kj::heapArray(userInfo.getIdentityId()), kj::mv(permissions)); + newUiSession( + userInfo, + kj::mv(sessionId), + sessionParams, + params.getContext(), + params.getTabId(), + SessionType::OFFER); context.getResults(capnp::MessageSize {2, 1}).setSession( - connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { - return kj::mv(session); - })); + connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { + return kj::mv(session); + })); + return kj::READY_NOW; } From 37e787ef0ddde5481b07a1c9e981d7f08017da02 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Mon, 17 Feb 2020 20:54:30 -0500 Subject: [PATCH 08/18] Bugfix: Don't double-increment the session id. --- src/sandstorm/sandstorm-http-bridge.c++ | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 2dc412e9e7..e62a9e0f06 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -2468,7 +2468,7 @@ public: UiSession::Client session = newUiSession( userInfo, - kj::str(sessionIdCounter++), + kj::mv(sessionId), sessionParams, params.getContext(), params.getTabId(), From b6000e8f478d4c8a3a4e84aeb1de6cf802f82a98 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Mon, 17 Feb 2020 22:31:10 -0500 Subject: [PATCH 09/18] Tweak the way we allocate the results for getSessionOffer/Request. It now doesn't crash until those methods are called, rather than on creation. I'm going to put this down for a bit and come back when I have the mental energy to sit down and understand what's going on. --- src/sandstorm/sandstorm-http-bridge.c++ | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index e62a9e0f06..df4af86d36 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1277,8 +1277,8 @@ public: const kj::StringPtr& id, capnp::List::Reader descriptor) { - kj::Own message; - auto results = message->getRoot(); + auto message = kj::heap(); + auto results = message->initRoot(); results.setRequestInfo(descriptor); requests.insert({kj::StringPtr(id), kj::mv(message)}); @@ -1289,8 +1289,8 @@ public: capnp::Capability::Client&& offer, PowerboxDescriptor::Reader descriptor) { - kj::Own message; - auto results = message->getRoot(); + auto message = kj::heap(); + auto results = message->initRoot(); results.setOffer(offer); results.setDescriptor(descriptor); From ad25e75712d8aed3b07c25998378c696cf0934e5 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Wed, 19 Feb 2020 00:18:33 -0500 Subject: [PATCH 10/18] Fix the segfault when fetching request/offer info. --- src/sandstorm/sandstorm-http-bridge.c++ | 41 +++++++++++-------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index df4af86d36..9b4231ee80 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1243,21 +1243,17 @@ public: return findInMap(sessions, id); } - kj::Maybe findOffer(const kj::StringPtr& id) { - KJ_IF_MAYBE(message, findInMap(offers, id)) { - SandstormHttpBridge::GetSessionOfferResults::Reader ret = - (*message)->getRoot(); - return ret; + kj::Maybe findOffer(const kj::StringPtr& id) { + KJ_IF_MAYBE(ret, findInMap(offers, id)) { + return ret->get(); } else { return nullptr; } } - kj::Maybe findRequest(const kj::StringPtr& id) { - KJ_IF_MAYBE(message, findInMap(requests, id)) { - SandstormHttpBridge::GetSessionRequestResults::Reader ret = - (*message)->getRoot(); - return ret; + kj::Maybe findRequest(const kj::StringPtr& id) { + KJ_IF_MAYBE(ret, findInMap(requests, id)) { + return ret->get(); } else { return nullptr; } @@ -1277,11 +1273,13 @@ public: const kj::StringPtr& id, capnp::List::Reader descriptor) { - auto message = kj::heap(); - auto results = message->initRoot(); + auto results = + capnp::MallocMessageBuilder() + .initRoot(); + results.setRequestInfo(descriptor); - requests.insert({kj::StringPtr(id), kj::mv(message)}); + requests.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); } void insertOffer( @@ -1289,12 +1287,14 @@ public: capnp::Capability::Client&& offer, PowerboxDescriptor::Reader descriptor) { - auto message = kj::heap(); - auto results = message->initRoot(); + auto results = + capnp::MallocMessageBuilder() + .initRoot(); + results.setOffer(offer); results.setDescriptor(descriptor); - offers.insert({kj::StringPtr(id), kj::mv(message)}); + offers.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); } private: @@ -1303,13 +1303,8 @@ private: kj::AutoCloseFd identitiesDir; kj::AutoCloseFd trashDir; std::map sessions; - - // Contains SandstormHttpBridge::GetSessionRequestResults. TODO(zenhack): figure - // out if there's a way to make this typed: - std::map> requests; - - // Contains SandstormHttpBridge::GetSessionOfferResults. - std::map> offers; + std::map> requests; + std::map> offers; struct IdentityRecord { IdentityRecord(const IdentityRecord& other) = delete; From 0cec8732a8f93c6432f8c9316645e1389037caec Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Thu, 20 Feb 2020 23:25:36 -0500 Subject: [PATCH 11/18] Reverse inverted comparison. Apparently I messed this up when factoring out the logic. --- src/sandstorm/sandstorm-http-bridge.c++ | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 9b4231ee80..99f2b090a1 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1121,7 +1121,7 @@ private: template kj::Maybe findInMap(std::map& map, const kj::StringPtr& id) { auto iter = map.find(id); - if(iter != map.end()) { + if(iter == map.end()) { return nullptr; } return iter->second; From d6ce8c4a6ad979878932e986170be6c8ffe01450 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Thu, 20 Feb 2020 23:30:18 -0500 Subject: [PATCH 12/18] Use kj::str in a couple new places. ...mostly in place of kj::mv. At least one of these was responsible for some memory corruption I was seeing. Some of the copies this introduces *may* be unnecessary, but it's probably better to have the copy in there than the unsafe cast, until it is demonstrably a problem. --- src/sandstorm/sandstorm-http-bridge.c++ | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 99f2b090a1..ec9ea08134 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1279,7 +1279,7 @@ public: results.setRequestInfo(descriptor); - requests.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); + requests.insert({kj::str(id), capnp::clone(results.asReader())}); } void insertOffer( @@ -2430,7 +2430,7 @@ public: auto userPermissions = userInfo.getPermissions(); return kj::heap(serverAddress, userInfo, sessionCtx, - bridgeContext, kj::mv(sessionId), + bridgeContext, kj::str(sessionId), kj::encodeHex(tabId), kj::heapString(sessionParams.getBasePath()), kj::heapString(sessionParams.getUserAgent()), @@ -2463,7 +2463,7 @@ public: UiSession::Client session = newUiSession( userInfo, - kj::mv(sessionId), + kj::str(sessionId), sessionParams, params.getContext(), params.getTabId(), @@ -2572,7 +2572,7 @@ public: UiSession::Client session = newUiSession( userInfo, - kj::mv(sessionId), + kj::str(sessionId), sessionParams, params.getContext(), params.getTabId(), From 2af079eba6a520079cd91a46f092c38c4785de76 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 21 Feb 2020 00:52:08 -0500 Subject: [PATCH 13/18] More kj::str when inserting into maps. --- src/sandstorm/sandstorm-http-bridge.c++ | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index ec9ea08134..9688ea359e 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1266,7 +1266,7 @@ public: } void insertSession(const kj::StringPtr& id, SessionContext::Client& session) { - sessions.insert({kj::StringPtr(id), session}); + sessions.insert({kj::str(id), session}); } void insertRequest( @@ -1294,7 +1294,7 @@ public: results.setOffer(offer); results.setDescriptor(descriptor); - offers.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); + offers.insert({kj::str(id), capnp::clone(results.asReader())}); } private: From 47524a51d824b07ef726f5d30be9bc3ffe0993a3 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Fri, 21 Feb 2020 03:21:54 -0500 Subject: [PATCH 14/18] Fix some broken logic re: ownership of map keys. See the comments for the current state of affairs. The code I'd written previously didn't have any coherent story here, and was *sometimes* working I think just because of luck wrt. what memory gets re-used by the allocator or doesn't. This needs more testing, but experimenting with my filesystem demo, grains, it gets further than anything before it. --- .../sandstorm-http-bridge-internal.capnp | 16 ++- src/sandstorm/sandstorm-http-bridge.c++ | 115 +++++++++++++----- 2 files changed, 99 insertions(+), 32 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge-internal.capnp b/src/sandstorm/sandstorm-http-bridge-internal.capnp index 64885d5946..a740699e61 100644 --- a/src/sandstorm/sandstorm-http-bridge-internal.capnp +++ b/src/sandstorm/sandstorm-http-bridge-internal.capnp @@ -23,6 +23,7 @@ using Identity = import "identity.capnp"; using WebSession = import "web-session.capnp".WebSession; using ApiSession = import "api-session.capnp".ApiSession; using AppPersistent = import "grain.capnp".AppPersistent; +using Powerbox = import "powerbox.capnp"; struct BridgeObjectId { # The object ID format used by sandstorm-http-bridge. @@ -59,8 +60,15 @@ interface BridgeHttpSession extends(ApiSession, AppPersistent(BridgeObjectId)) { const bridgeRequestSessionHtml :Text = embed "sandstorm-http-bridge-request.html"; -enum SessionType { - normal @0; - request @1; - offer @2; +struct SessionInfo { + union { + normal @0 :Void; + request :group { + requestInfo @1 :List(Powerbox.PowerboxDescriptor); + } + offer :group { + offer @2 :Capability; + descriptor @3 :Powerbox.PowerboxDescriptor; + } + } } diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 9688ea359e..3fd2e25a59 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -20,6 +20,7 @@ // that server and then redirect incoming requests to it over standard HTTP on // the loopback network interface. +#include #include #include #include @@ -1266,20 +1267,18 @@ public: } void insertSession(const kj::StringPtr& id, SessionContext::Client& session) { - sessions.insert({kj::str(id), session}); + sessions.insert({kj::StringPtr(id), session}); } void insertRequest( const kj::StringPtr& id, capnp::List::Reader descriptor) { - auto results = - capnp::MallocMessageBuilder() - .initRoot(); - + auto msg = capnp::MallocMessageBuilder(); + auto results = msg.initRoot(); results.setRequestInfo(descriptor); - requests.insert({kj::str(id), capnp::clone(results.asReader())}); + requests.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); } void insertOffer( @@ -1287,14 +1286,13 @@ public: capnp::Capability::Client&& offer, PowerboxDescriptor::Reader descriptor) { - auto results = - capnp::MallocMessageBuilder() - .initRoot(); + auto msg = capnp::MallocMessageBuilder(); + auto results = msg.initRoot(); results.setOffer(offer); results.setDescriptor(descriptor); - offers.insert({kj::str(id), capnp::clone(results.asReader())}); + offers.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); } private: @@ -1302,6 +1300,15 @@ private: spk::BridgeConfig::Reader config; kj::AutoCloseFd identitiesDir; kj::AutoCloseFd trashDir; + + // These maps store information relating to active sessions. `sessions` always has an entry + // for any active session that is handled by the app. `requests` and `offers` will only + // have an entry for request and offer sessions, respectively. + // + // In all cases the keys are owned by the session object, which is responsible for deleting + // the session from these maps (via eraseSession) on cleanup. For `sessions`, the value + // is also owned by the session. For `requests` and `offers`, the value is owned by the map + // itself. std::map sessions; std::map> requests; std::map> offers; @@ -1421,7 +1428,7 @@ public: kj::String&& rootPath, kj::String&& permissions, kj::Maybe remoteAddress, kj::Maybe>&& apiInfo, - SessionType sessionType) + SessionInfo::Reader sessionInfo) : serverAddr(serverAddr), sessionContext(kj::mv(sessionContext)), bridgeContext(bridgeContext), @@ -1438,12 +1445,32 @@ public: rootPath(kj::mv(rootPath)), remoteAddress(kj::mv(remoteAddress)), apiInfo(kj::mv(apiInfo)), - sessionType(sessionType) { + sessionType(sessionInfo.which()) { if (userInfo.hasIdentityId()) { userId = textIdentityId(userInfo.getIdentityId()); } if (this->sessionId != nullptr) { bridgeContext.insertSession(kj::StringPtr(this->sessionId), this->sessionContext); + switch(sessionInfo.which()) { + case SessionInfo::NORMAL: + break; + case SessionInfo::REQUEST: + bridgeContext.insertRequest( + kj::StringPtr(this->sessionId), + sessionInfo.getRequest().getRequestInfo()); + break; + case SessionInfo::OFFER: + { + auto offerInfo = sessionInfo.getOffer(); + bridgeContext.insertOffer( + kj::StringPtr(this->sessionId), + offerInfo.getOffer(), + offerInfo.getDescriptor()); + } + break; + default: + KJ_FAIL_ASSERT("Unknown session type."); + } } } @@ -1705,7 +1732,7 @@ private: spk::BridgeConfig::Reader config; kj::Maybe remoteAddress; kj::Maybe> apiInfo; - SessionType sessionType; + SessionInfo::Which sessionType; kj::String makeHeaders(kj::StringPtr method, kj::StringPtr path, WebSession::Context::Reader context, @@ -1768,11 +1795,25 @@ private: } } { - capnp::EnumSchema schema = capnp::Schema::from(); - uint typeValue = static_cast(sessionType); - auto enumerants = schema.getEnumerants(); - lines.add(kj::str("X-Sandstorm-Session-Type: ", - enumerants[typeValue].getProto().getName())); + // TODO(zenhack): there's probably an existing method to get the name + // of a variant; look it up and just do that instead of doing all + // this manually: + auto setHeader = [&](const char *value) { + lines.add(kj::str("X-Sandstorm-Session-Type: ", value)); + }; + switch(sessionType) { + case SessionInfo::Which::NORMAL: + setHeader("normal"); + break; + case SessionInfo::Which::REQUEST: + setHeader("request"); + break; + case SessionInfo::Which::OFFER: + setHeader("offer"); + break; + default: + KJ_FAIL_ASSERT("Unknown session type."); + } } lines.add(kj::str("X-Sandstorm-Permissions: ", permissions)); if (basePath.size() > 0) { @@ -2008,6 +2049,10 @@ WebSession::Client newPowerboxApiSession( userInfo.setIdentityId(httpApi.getIdentityId()); userInfo.setIdentity(kj::mv(identity)); + auto msg = capnp::MallocMessageBuilder(); + auto sessionInfo = msg.initRoot(); + sessionInfo.setNormal(); + return WebSession::Client( kj::heap(serverAddress, userInfo, nullptr, bridgeContext, nullptr, nullptr, @@ -2015,7 +2060,7 @@ WebSession::Client newPowerboxApiSession( kj::str(httpApi.getPath(), '/'), bridgeContext.formatPermissions(httpApi.getPermissions()), nullptr, kj::mv(httpApi), - SessionType::NORMAL)); + sessionInfo)); }); }); } @@ -2425,7 +2470,7 @@ public: WebSession::Params::Reader sessionParams, SessionContext::Client sessionCtx, kj::ArrayPtr tabId, - SessionType sessionType) { + SessionInfo::Reader sessionInfo) { auto userPermissions = userInfo.getPermissions(); return @@ -2438,7 +2483,7 @@ public: kj::heapString("/"), bridgeContext.formatPermissions(userPermissions), nullptr, nullptr, - sessionType); + sessionInfo); } @@ -2457,9 +2502,12 @@ public: } if (sessionType == capnp::typeId()) { - auto userPermissions = userInfo.getPermissions(); auto sessionParams = params.getSessionParams().getAs(); + auto msg = capnp::MallocMessageBuilder(); + auto sessionInfo = msg.initRoot(); + sessionInfo.setNormal(); + UiSession::Client session = newUiSession( userInfo, @@ -2467,7 +2515,7 @@ public: sessionParams, params.getContext(), params.getTabId(), - SessionType::NORMAL); + sessionInfo); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { @@ -2481,6 +2529,9 @@ public: addr = addressToString(sessionParams.getRemoteAddress()); } + auto msg = capnp::MallocMessageBuilder(); + auto sessionInfo = msg.initRoot(); + sessionInfo.setNormal(); UiSession::Client session = kj::heap(serverAddress, userInfo, params.getContext(), bridgeContext, kj::str(sessionIdCounter++), @@ -2489,7 +2540,7 @@ public: kj::heapString(config.getApiPath()), bridgeContext.formatPermissions(userPermissions), kj::mv(addr), nullptr, - SessionType::NORMAL); + sessionInfo); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { @@ -2542,8 +2593,12 @@ public: // the request. auto sessionId = kj::str(sessionIdCounter++); - bridgeContext.insertRequest(kj::str(sessionId), requestInfo); auto sessionParams = params.getSessionParams().getAs(); + + auto msg = capnp::MallocMessageBuilder(); + auto sessionInfo = msg.initRoot(); + sessionInfo.initRequest().setRequestInfo(requestInfo); + UiSession::Client session = newUiSession( userInfo, @@ -2551,7 +2606,7 @@ public: sessionParams, params.getContext(), params.getTabId(), - SessionType::REQUEST); + sessionInfo); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { @@ -2567,7 +2622,11 @@ public: auto userInfo = params.getUserInfo(); auto sessionParams = params.getSessionParams().getAs(); - bridgeContext.insertOffer(kj::str(sessionId), params.getOffer(), params.getDescriptor()); + auto msg = capnp::MallocMessageBuilder(); + auto sessionInfo = msg.initRoot(); + auto offerInfo = sessionInfo.initOffer(); + offerInfo.setOffer(params.getOffer()); + offerInfo.setDescriptor(params.getDescriptor()); UiSession::Client session = newUiSession( @@ -2576,7 +2635,7 @@ public: sessionParams, params.getContext(), params.getTabId(), - SessionType::OFFER); + sessionInfo); context.getResults(capnp::MessageSize {2, 1}).setSession( connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable { From 719e6f95fe5d2d87d22114b5efc9ed7604d8aae4 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Sat, 22 Feb 2020 14:30:46 -0500 Subject: [PATCH 15/18] Remove left over include from print debugging. --- src/sandstorm/sandstorm-http-bridge.c++ | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 3fd2e25a59..561ef10d11 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -20,7 +20,6 @@ // that server and then redirect incoming requests to it over standard HTTP on // the loopback network interface. -#include #include #include #include From 383963dabcb00d0934d602d25d76146b226b0b10 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Sat, 22 Feb 2020 14:35:07 -0500 Subject: [PATCH 16/18] Fix build error due to merge. --- src/sandstorm/sandstorm-http-bridge.c++ | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index 561ef10d11..f1df239efa 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -2510,7 +2510,7 @@ public: UiSession::Client session = newUiSession( userInfo, - kj::str(sessionId), + kj::str(sessionIdCounter++), sessionParams, params.getContext(), params.getTabId(), From d99cfa12bf2b3438be312875d419bfeef6011fb2 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Sat, 22 Feb 2020 15:23:26 -0500 Subject: [PATCH 17/18] bridge: use one map for session info ...instead of three, per @kentonv's suggestion. --- src/sandstorm/sandstorm-http-bridge.c++ | 133 +++++++++--------------- 1 file changed, 52 insertions(+), 81 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index f1df239efa..a256eaae4d 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1239,21 +1239,17 @@ public: } } - kj::Maybe findSession(const kj::StringPtr& id) { - return findInMap(sessions, id); - } - - kj::Maybe findOffer(const kj::StringPtr& id) { - KJ_IF_MAYBE(ret, findInMap(offers, id)) { - return ret->get(); + kj::Maybe findSessionContext(const kj::StringPtr& id) { + KJ_IF_MAYBE(record, findInMap(sessions, id)) { + return record->sessionCtx; } else { return nullptr; } } - kj::Maybe findRequest(const kj::StringPtr& id) { - KJ_IF_MAYBE(ret, findInMap(requests, id)) { - return ret->get(); + kj::Maybe&> findSessionInfo(const kj::StringPtr& id) { + KJ_IF_MAYBE(record, findInMap(sessions, id)) { + return record->sessionInfo; } else { return nullptr; } @@ -1261,37 +1257,13 @@ public: void eraseSession(const kj::StringPtr& id) { sessions.erase(id); - offers.erase(id); - requests.erase(id); - } - - void insertSession(const kj::StringPtr& id, SessionContext::Client& session) { - sessions.insert({kj::StringPtr(id), session}); - } - - void insertRequest( - const kj::StringPtr& id, - capnp::List::Reader descriptor) { - - auto msg = capnp::MallocMessageBuilder(); - auto results = msg.initRoot(); - results.setRequestInfo(descriptor); - - requests.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); } - void insertOffer( - const kj::StringPtr& id, - capnp::Capability::Client&& offer, - PowerboxDescriptor::Reader descriptor) { - - auto msg = capnp::MallocMessageBuilder(); - auto results = msg.initRoot(); - - results.setOffer(offer); - results.setDescriptor(descriptor); - - offers.insert({kj::StringPtr(id), capnp::clone(results.asReader())}); + void insertSession(const kj::StringPtr& id, SessionContext::Client& session, kj::Own& sessionInfo) { + sessions.insert({ + kj::StringPtr(id), + SessionRecord {session, sessionInfo} + }); } private: @@ -1300,17 +1272,14 @@ private: kj::AutoCloseFd identitiesDir; kj::AutoCloseFd trashDir; - // These maps store information relating to active sessions. `sessions` always has an entry - // for any active session that is handled by the app. `requests` and `offers` will only - // have an entry for request and offer sessions, respectively. - // - // In all cases the keys are owned by the session object, which is responsible for deleting - // the session from these maps (via eraseSession) on cleanup. For `sessions`, the value - // is also owned by the session. For `requests` and `offers`, the value is owned by the map - // itself. - std::map sessions; - std::map> requests; - std::map> offers; + struct SessionRecord { + SessionRecord(const SessionRecord& other) = delete; + SessionRecord(SessionRecord&& other) = default; + + SessionContext::Client& sessionCtx; + kj::Own& sessionInfo; + }; + std::map sessions; struct IdentityRecord { IdentityRecord(const IdentityRecord& other) = delete; @@ -1444,32 +1413,15 @@ public: rootPath(kj::mv(rootPath)), remoteAddress(kj::mv(remoteAddress)), apiInfo(kj::mv(apiInfo)), - sessionType(sessionInfo.which()) { + sessionInfo(capnp::clone(sessionInfo)) { if (userInfo.hasIdentityId()) { userId = textIdentityId(userInfo.getIdentityId()); } if (this->sessionId != nullptr) { - bridgeContext.insertSession(kj::StringPtr(this->sessionId), this->sessionContext); - switch(sessionInfo.which()) { - case SessionInfo::NORMAL: - break; - case SessionInfo::REQUEST: - bridgeContext.insertRequest( - kj::StringPtr(this->sessionId), - sessionInfo.getRequest().getRequestInfo()); - break; - case SessionInfo::OFFER: - { - auto offerInfo = sessionInfo.getOffer(); - bridgeContext.insertOffer( - kj::StringPtr(this->sessionId), - offerInfo.getOffer(), - offerInfo.getDescriptor()); - } - break; - default: - KJ_FAIL_ASSERT("Unknown session type."); - } + bridgeContext.insertSession( + kj::StringPtr(this->sessionId), + this->sessionContext, + this->sessionInfo); } } @@ -1731,7 +1683,7 @@ private: spk::BridgeConfig::Reader config; kj::Maybe remoteAddress; kj::Maybe> apiInfo; - SessionInfo::Which sessionType; + kj::Own sessionInfo; kj::String makeHeaders(kj::StringPtr method, kj::StringPtr path, WebSession::Context::Reader context, @@ -1800,7 +1752,7 @@ private: auto setHeader = [&](const char *value) { lines.add(kj::str("X-Sandstorm-Session-Type: ", value)); }; - switch(sessionType) { + switch(sessionInfo->which()) { case SessionInfo::Which::NORMAL: setHeader("normal"); break; @@ -2359,7 +2311,7 @@ public: kj::Promise getSessionContext(GetSessionContextContext context) override { auto id = context.getParams().getId(); - KJ_IF_MAYBE(value, bridgeContext.findSession(id)) { + KJ_IF_MAYBE(value, bridgeContext.findSessionContext(id)) { context.getResults().setContext(*value); } else { KJ_FAIL_ASSERT("Session ID not found", id); @@ -2369,20 +2321,39 @@ public: kj::Promise getSessionOffer(GetSessionOfferContext context) override { auto id = context.getParams().getId(); - KJ_IF_MAYBE(value, bridgeContext.findOffer(id)) { - context.setResults(*value); + KJ_IF_MAYBE(sessionInfo, bridgeContext.findSessionInfo(id)) { + switch((*sessionInfo)->which()) { + case SessionInfo::OFFER: + { + auto offerInfo = (*sessionInfo)->getOffer(); + auto results = context.initResults(); + results.setOffer(offerInfo.getOffer()); + results.setDescriptor(offerInfo.getDescriptor()); + } + break; + default: + KJ_FAIL_ASSERT("Session ID ", id, " is not an offer session."); + } } else { - KJ_FAIL_ASSERT("Session ID not found; maybe this isn't an offer session?"); + KJ_FAIL_ASSERT("Session ID ", id, " not found"); } return kj::READY_NOW; } kj::Promise getSessionRequest(GetSessionRequestContext context) override { auto id = context.getParams().getId(); - KJ_IF_MAYBE(value, bridgeContext.findRequest(id)) { - context.setResults(*value); + KJ_IF_MAYBE(sessionInfo, bridgeContext.findSessionInfo(id)) { + switch((*sessionInfo)->which()) { + case SessionInfo::REQUEST: + context + .initResults() + .setRequestInfo((*sessionInfo)->getRequest().getRequestInfo()); + break; + default: + KJ_FAIL_ASSERT("Session ID ", id, " is not a request session."); + } } else { - KJ_FAIL_ASSERT("Session ID not found; maybe this isn't a request session?"); + KJ_FAIL_ASSERT("Session ID ", id, " not found"); } return kj::READY_NOW; } From 66c7d7a18d2b2e7da94f5275785b858f54754e37 Mon Sep 17 00:00:00 2001 From: Ian Denhardt Date: Sat, 22 Feb 2020 15:56:11 -0500 Subject: [PATCH 18/18] Get rid of redundant kj::Own reference. Readers are already reference types, so let's just store that directly. --- src/sandstorm/sandstorm-http-bridge.c++ | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sandstorm/sandstorm-http-bridge.c++ b/src/sandstorm/sandstorm-http-bridge.c++ index a256eaae4d..68d654dd08 100644 --- a/src/sandstorm/sandstorm-http-bridge.c++ +++ b/src/sandstorm/sandstorm-http-bridge.c++ @@ -1247,7 +1247,7 @@ public: } } - kj::Maybe&> findSessionInfo(const kj::StringPtr& id) { + kj::Maybe findSessionInfo(const kj::StringPtr& id) { KJ_IF_MAYBE(record, findInMap(sessions, id)) { return record->sessionInfo; } else { @@ -1259,7 +1259,7 @@ public: sessions.erase(id); } - void insertSession(const kj::StringPtr& id, SessionContext::Client& session, kj::Own& sessionInfo) { + void insertSession(const kj::StringPtr& id, SessionContext::Client& session, SessionInfo::Reader sessionInfo) { sessions.insert({ kj::StringPtr(id), SessionRecord {session, sessionInfo} @@ -1277,7 +1277,7 @@ private: SessionRecord(SessionRecord&& other) = default; SessionContext::Client& sessionCtx; - kj::Own& sessionInfo; + SessionInfo::Reader sessionInfo; }; std::map sessions; @@ -1421,7 +1421,7 @@ public: bridgeContext.insertSession( kj::StringPtr(this->sessionId), this->sessionContext, - this->sessionInfo); + *this->sessionInfo); } } @@ -2322,10 +2322,10 @@ public: kj::Promise getSessionOffer(GetSessionOfferContext context) override { auto id = context.getParams().getId(); KJ_IF_MAYBE(sessionInfo, bridgeContext.findSessionInfo(id)) { - switch((*sessionInfo)->which()) { + switch(sessionInfo->which()) { case SessionInfo::OFFER: { - auto offerInfo = (*sessionInfo)->getOffer(); + auto offerInfo = sessionInfo->getOffer(); auto results = context.initResults(); results.setOffer(offerInfo.getOffer()); results.setDescriptor(offerInfo.getDescriptor()); @@ -2343,11 +2343,11 @@ public: kj::Promise getSessionRequest(GetSessionRequestContext context) override { auto id = context.getParams().getId(); KJ_IF_MAYBE(sessionInfo, bridgeContext.findSessionInfo(id)) { - switch((*sessionInfo)->which()) { + switch(sessionInfo->which()) { case SessionInfo::REQUEST: context .initResults() - .setRequestInfo((*sessionInfo)->getRequest().getRequestInfo()); + .setRequestInfo(sessionInfo->getRequest().getRequestInfo()); break; default: KJ_FAIL_ASSERT("Session ID ", id, " is not a request session.");