Skip to content

Commit

Permalink
Fix some broken logic re: ownership of map keys.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zenhack
zenhack committed Feb 21, 2020
1 parent 4114923 commit b1597f7
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 32 deletions.
16 changes: 12 additions & 4 deletions src/sandstorm/sandstorm-http-bridge-internal.capnp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}
}
115 changes: 87 additions & 28 deletions src/sandstorm/sandstorm-http-bridge.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// that server and then redirect incoming requests to it over standard HTTP on
// the loopback network interface.

#include <iostream>
#include <kj/main.h>
#include <kj/debug.h>
#include <kj/async-io.h>
Expand Down Expand Up @@ -1208,42 +1209,48 @@ 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<PowerboxDescriptor>::Reader descriptor) {

auto results =
capnp::MallocMessageBuilder()
.initRoot<SandstormHttpBridge::GetSessionRequestResults>();

auto msg = capnp::MallocMessageBuilder();
auto results = msg.initRoot<SandstormHttpBridge::GetSessionRequestResults>();
results.setRequestInfo(descriptor);

requests.insert({kj::str(id), capnp::clone(results.asReader())});
requests.insert({kj::StringPtr(id), capnp::clone(results.asReader())});
}

void insertOffer(
const kj::StringPtr& id,
capnp::Capability::Client&& offer,
PowerboxDescriptor::Reader descriptor) {

auto results =
capnp::MallocMessageBuilder()
.initRoot<SandstormHttpBridge::GetSessionOfferResults>();
auto msg = capnp::MallocMessageBuilder();
auto results = msg.initRoot<SandstormHttpBridge::GetSessionOfferResults>();

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:
SandstormApi<BridgeObjectId>::Client apiCap;
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<kj::StringPtr, SessionContext::Client&> sessions;
std::map<kj::StringPtr, kj::Own<SandstormHttpBridge::GetSessionRequestResults::Reader>> requests;
std::map<kj::StringPtr, kj::Own<SandstormHttpBridge::GetSessionOfferResults::Reader>> offers;
Expand Down Expand Up @@ -1363,7 +1370,7 @@ public:
kj::String&& rootPath, kj::String&& permissions,
kj::Maybe<kj::String> remoteAddress,
kj::Maybe<OwnCapnp<BridgeObjectId::HttpApi>>&& apiInfo,
SessionType sessionType)
SessionInfo::Reader sessionInfo)
: serverAddr(serverAddr),
sessionContext(kj::mv(sessionContext)),
bridgeContext(bridgeContext),
Expand All @@ -1380,12 +1387,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.");
}
}
}

Expand Down Expand Up @@ -1647,7 +1674,7 @@ private:
spk::BridgeConfig::Reader config;
kj::Maybe<kj::String> remoteAddress;
kj::Maybe<OwnCapnp<BridgeObjectId::HttpApi>> apiInfo;
SessionType sessionType;
SessionInfo::Which sessionType;

kj::String makeHeaders(kj::StringPtr method, kj::StringPtr path,
WebSession::Context::Reader context,
Expand Down Expand Up @@ -1710,11 +1737,25 @@ private:
}
}
{
capnp::EnumSchema schema = capnp::Schema::from<SessionType>();
uint typeValue = static_cast<uint>(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) {
Expand Down Expand Up @@ -1950,14 +1991,18 @@ WebSession::Client newPowerboxApiSession(
userInfo.setIdentityId(httpApi.getIdentityId());
userInfo.setIdentity(kj::mv(identity));

auto msg = capnp::MallocMessageBuilder();
auto sessionInfo = msg.initRoot<SessionInfo>();
sessionInfo.setNormal();

return WebSession::Client(
kj::heap<WebSessionImpl>(serverAddress, userInfo, nullptr,
bridgeContext, nullptr, nullptr,
nullptr, nullptr, nullptr,
kj::str(httpApi.getPath(), '/'),
bridgeContext.formatPermissions(httpApi.getPermissions()),
nullptr, kj::mv(httpApi),
SessionType::NORMAL));
sessionInfo));
});
});
}
Expand Down Expand Up @@ -2341,7 +2386,7 @@ public:
WebSession::Params::Reader sessionParams,
SessionContext::Client sessionCtx,
kj::ArrayPtr<const kj::byte> tabId,
SessionType sessionType) {
SessionInfo::Reader sessionInfo) {

auto userPermissions = userInfo.getPermissions();
return
Expand All @@ -2354,7 +2399,7 @@ public:
kj::heapString("/"),
bridgeContext.formatPermissions(userPermissions),
nullptr, nullptr,
sessionType);
sessionInfo);

}

Expand All @@ -2373,17 +2418,20 @@ public:
}

if (sessionType == capnp::typeId<WebSession>()) {
auto userPermissions = userInfo.getPermissions();
auto sessionParams = params.getSessionParams().getAs<WebSession::Params>();

auto msg = capnp::MallocMessageBuilder();
auto sessionInfo = msg.initRoot<SessionInfo>();
sessionInfo.setNormal();

UiSession::Client session =
newUiSession(
userInfo,
kj::str(sessionIdCounter++),
sessionParams,
params.getContext(),
params.getTabId(),
SessionType::NORMAL);
sessionInfo);

context.getResults(capnp::MessageSize {2, 1}).setSession(
connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable {
Expand All @@ -2397,6 +2445,9 @@ public:
addr = addressToString(sessionParams.getRemoteAddress());
}

auto msg = capnp::MallocMessageBuilder();
auto sessionInfo = msg.initRoot<SessionInfo>();
sessionInfo.setNormal();
UiSession::Client session =
kj::heap<WebSessionImpl>(serverAddress, userInfo, params.getContext(),
bridgeContext, kj::str(sessionIdCounter++),
Expand All @@ -2405,7 +2456,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 {
Expand Down Expand Up @@ -2458,16 +2509,20 @@ public:
// the request.
auto sessionId = kj::str(sessionIdCounter++);

bridgeContext.insertRequest(kj::str(sessionId), requestInfo);
auto sessionParams = params.getSessionParams().getAs<WebSession::Params>();

auto msg = capnp::MallocMessageBuilder();
auto sessionInfo = msg.initRoot<SessionInfo>();
sessionInfo.initRequest().setRequestInfo(requestInfo);

UiSession::Client session =
newUiSession(
userInfo,
kj::str(sessionId),
sessionParams,
params.getContext(),
params.getTabId(),
SessionType::REQUEST);
sessionInfo);

context.getResults(capnp::MessageSize {2, 1}).setSession(
connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable {
Expand All @@ -2483,7 +2538,11 @@ public:
auto userInfo = params.getUserInfo();
auto sessionParams = params.getSessionParams().getAs<WebSession::Params>();

bridgeContext.insertOffer(kj::str(sessionId), params.getOffer(), params.getDescriptor());
auto msg = capnp::MallocMessageBuilder();
auto sessionInfo = msg.initRoot<SessionInfo>();
auto offerInfo = sessionInfo.initOffer();
offerInfo.setOffer(params.getOffer());
offerInfo.setDescriptor(params.getDescriptor());

UiSession::Client session =
newUiSession(
Expand All @@ -2492,7 +2551,7 @@ public:
sessionParams,
params.getContext(),
params.getTabId(),
SessionType::OFFER);
sessionInfo);

context.getResults(capnp::MessageSize {2, 1}).setSession(
connectPromise.addBranch().then([KJ_MVCAP(session)]() mutable {
Expand Down

0 comments on commit b1597f7

Please sign in to comment.