Skip to content

Commit

Permalink
Use softdelete to destroy session to prevent race condition and re-save
Browse files Browse the repository at this point in the history
  • Loading branch information
0x0ece authored and nykula committed Jun 11, 2021
1 parent 7afbbe6 commit b29dbb6
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 18 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"supertest": "^3.1.0",
"test-all-versions": "^4.1.0",
"tslint": "^5.8.0",
"typeorm": "^0.1.4",
"typeorm": "^0.2.34",
"typescript": "^3.8.3"
},
"dependencies": {
Expand Down
61 changes: 57 additions & 4 deletions src/app/TypeormStore/TypeormStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Column,
Connection,
createConnection,
DeleteDateColumn,
Entity,
Index,
PrimaryColumn,
Expand Down Expand Up @@ -128,7 +129,32 @@ test("touches, handling error", async (t) => {

await ctx.componentWillUnmount();

await ctx.request.get("/views").expect(/database.*closed/i);
await ctx.request.get("/views").expect(/database.*not\ established/i);
});

test("race condition", async (t) => {
const { request, repository, blockLogout, unblockReq1 } = t.context as Test;

t.is((await request.post("/views")).body, 1);
t.is((await request.post("/views")).body, 2);

// hold logout until triggered by req to /race
blockLogout.then(async () => {
await request.delete("/views");
// let /race continue
unblockReq1();
});
// call to /race returns as expected as logout happens in between
// because session.views was 2, race increments and returns 3
t.is((await request.post("/race")).body, 3);

// the record in the db should be deleted and should NOT be updated
// i.e. views should still be at 2, not re-saved to 3
const records = await repository.find({ withDeleted: true });
t.is(JSON.parse(records[0].json).views, 2);

// session was deleted, so a new call results in a new session
t.is((await request.post("/views")).body, 1);
});

test.afterEach(async (t) => {
Expand All @@ -146,6 +172,9 @@ class Session implements ISession {
@PrimaryColumn("varchar", { length: 255 })
public id = "";

@DeleteDateColumn()
public destroyedAt?: Date;

@Column("text")
public json = "";
}
Expand All @@ -159,16 +188,29 @@ class Test {

public ttl = 2;

public unblockReq1: any = null;
public unblockLogout: any = null;
public blockReq1: any = null;
public blockLogout: any = null;

private connection: Connection | undefined;

public async componentDidMount() {
this.connection = await createConnection({
database: ":memory:",
entities: [Session],
// logging: ["query", "error"],
synchronize: true,
type: "sqlite",
});

this.blockReq1 = new Promise((resolve, _) => {
this.unblockReq1 = resolve;
});
this.blockLogout = new Promise((resolve, _) => {
this.unblockLogout = resolve;
});

this.repository = this.connection.getRepository(Session);

this.express.use(
Expand All @@ -195,15 +237,26 @@ class Test {
this.express.get("/views", (req, res) => {
const session = nullthrows(req.session);

res.json(session.views || 0);
res.json((session as any).views || 0);
});

this.express.post("/views", (req, res) => {
const session = nullthrows(req.session);

session.views = (session.views || 0) + 1;
(session as any).views = ((session as any).views || 0) + 1;

res.json((session as any).views);
});

this.express.post("/race", async (req, res) => {
const session = nullthrows(req.session);

(session as any).views = ((session as any).views || 0) + 1;

res.json(session.views);
this.unblockLogout();
this.blockReq1.then(() => {
return res.json((session as any).views);
});
});
}

Expand Down
32 changes: 22 additions & 10 deletions src/app/TypeormStore/TypeormStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class TypeormStore extends Store {
}
> = {},
) {
super(options);
super(options as any);
this.cleanupLimit = options.cleanupLimit;
if (options.limitSubquery !== undefined) {
this.limitSubquery = options.limitSubquery;
Expand Down Expand Up @@ -130,21 +130,33 @@ export class TypeormStore extends Store {
)
: Promise.resolve()
)
.then(() =>
this.repository.save({
expiredAt: Date.now() + ttl * 1000,
id: sid,
json,
}),
)
// @ts-ignore
.then(async () => {
try {
await this.repository.findOneOrFail({ id: sid }, { withDeleted: true });
this.repository.update({
destroyedAt: null,
id: sid,
} as any, {
expiredAt: Date.now() + ttl * 1000,
json,
});
} catch (_) {
this.repository.insert({
expiredAt: Date.now() + ttl * 1000,
id: sid,
json,
});
}
})
.then(() => {
this.debug("SET complete");

if (fn) {
fn();
}
})
.catch((er) => {
.catch((er: any) => {
if (fn) {
fn(er);
}
Expand All @@ -159,7 +171,7 @@ export class TypeormStore extends Store {
public destroy = (sid: string | string[], fn?: (error?: any) => void) => {
this.debug('DEL "%s"', sid);

Promise.all((Array.isArray(sid) ? sid : [sid]).map((x) => this.repository.delete({ id: x })))
Promise.all((Array.isArray(sid) ? sid : [sid]).map((x) => this.repository.softDelete({ id: x })))
.then(() => {
if (fn) {
fn();
Expand Down
10 changes: 7 additions & 3 deletions src/app/TypeormStore/onError.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Column,
Connection,
createConnection,
DeleteDateColumn,
Entity,
Index,
PrimaryColumn,
Expand All @@ -27,6 +28,9 @@ class Session implements ISession {
@PrimaryColumn("varchar", { length: 255 })
public id = "";

@DeleteDateColumn()
public destroyedAt?: Date;

@Column("text")
public json = "";
}
Expand All @@ -49,12 +53,12 @@ class Test {
public route() {
this.express.get("/views", (req, res) => {
const session = nullthrows(req.session);
res.json(session.views || 0);
res.json((session as any).views || 0);
});
this.express.post("/views", (req, res) => {
const session = nullthrows(req.session);
session.views = (session.views || 0) + 1;
res.json(session.views);
(session as any).views = ((session as any).views || 0) + 1;
res.json((session as any).views);
});
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/domain/Session/ISession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@ export interface ISession {

id: string;

destroyedAt?: Date;

json: string;
}

0 comments on commit b29dbb6

Please sign in to comment.