From ee928ed6a931bb78549a8b9969589038d978c335 Mon Sep 17 00:00:00 2001 From: Tanner Date: Wed, 6 May 2020 12:38:03 -0400 Subject: [PATCH] fix invalid session id handling (#2347) --- .../Vapor/Sessions/SessionsMiddleware.swift | 8 ++- Tests/VaporTests/SessionTests.swift | 52 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/Sources/Vapor/Sessions/SessionsMiddleware.swift b/Sources/Vapor/Sessions/SessionsMiddleware.swift index 731adc563c..67cba33e83 100644 --- a/Sources/Vapor/Sessions/SessionsMiddleware.swift +++ b/Sources/Vapor/Sessions/SessionsMiddleware.swift @@ -42,7 +42,13 @@ public final class SessionsMiddleware: Middleware { // A cookie value exists, get the session for it. let id = SessionID(string: cookieValue.string) return self.session.readSession(id, for: request).flatMap { data in - request._sessionCache.session = .init(id: id, data: data ?? .init()) + if let data = data { + // Session found, restore data and id. + request._sessionCache.session = .init(id: id, data: data) + } else { + // Session id not found, create new session. + request._sessionCache.session = .init() + } return next.respond(to: request).flatMap { res in return self.addCookies(to: res, for: request) } diff --git a/Tests/VaporTests/SessionTests.swift b/Tests/VaporTests/SessionTests.swift index 4050a8c91f..5935a9af1b 100644 --- a/Tests/VaporTests/SessionTests.swift +++ b/Tests/VaporTests/SessionTests.swift @@ -70,6 +70,58 @@ final class SessionTests: XCTestCase { } } + func testInvalidCookie() throws { + let app = Application(.testing) + defer { app.shutdown() } + + // Configure sessions. + app.sessions.use(.memory) + app.middleware.use(app.sessions.middleware) + + // Adds data to the request session. + app.get("set") { req -> HTTPStatus in + req.session.data["foo"] = "bar" + return .ok + } + + // Fetches data from the request session. + app.get("get") { req -> String in + guard let foo = req.session.data["foo"] else { + throw Abort(.badRequest) + } + return foo + } + + + // Test accessing session with no cookie. + try app.test(.GET, "get") { res in + XCTAssertEqual(res.status, .badRequest) + } + + // Test setting session with invalid cookie. + var newCookie: HTTPCookies.Value? + try app.test(.GET, "set", beforeRequest: { req in + req.headers.cookie = ["vapor-session": "foo"] + }, afterResponse: { res in + // We should get a new cookie back. + newCookie = res.headers.setCookie?["vapor-session"] + XCTAssertNotNil(newCookie) + // That is not the same as the invalid cookie we sent. + XCTAssertNotEqual(newCookie?.string, "foo") + XCTAssertEqual(res.status, .ok) + }) + + // Test accessing newly created session. + try app.test(.GET, "get", beforeRequest: { req in + // Pass cookie from previous request. + req.headers.cookie = ["vapor-session": newCookie!] + }, afterResponse: { res in + // Session access should be successful. + XCTAssertEqual(res.body.string, "bar") + XCTAssertEqual(res.status, .ok) + }) + } + func testCookieQuotes() throws { var headers = HTTPHeaders() headers.replaceOrAdd(name: .cookie, value: #"foo= "+cookie/value" "#)