Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InMemorySessionStorage fails with exception if the session id is invalid #69

Closed
audax opened this issue Jan 24, 2017 · 0 comments
Closed
Labels

Comments

@audax
Copy link

audax commented Jan 24, 2017

The contract of the interface of session storages defines that SessiosStorage.lookup should only call the consumer if the session can be found and otherwise fail silently.

The InMemorySessionStorage throws an IllegalArgumentException is the session can not be found and thus crashes the pipeline.

https://github.com/Kotlin/ktor/blob/master/ktor-features/ktor-server-sessions/src/org/jetbrains/ktor/sessions/SessionStorage.kt#L46


private class InMemorySessionStorage : SessionStorage {
    private val sessions = ConcurrentHashMap<String, ByteArray>()

    override fun save(id: String, contentProvider: (OutputStream) -> Unit): Future<Unit> {
        val baos = ByteArrayOutputStream()
        contentProvider(baos)
        sessions[id] = baos.toByteArray()
        return CompletableFuture.completedFuture(Unit)
    }

    override fun <R> read(id: String, consumer: (InputStream) -> R): Future<R> = CompletableFuture<R>().apply {
        sessions[id]?.let { bytes -> complete(consumer(bytes.inputStream())) } ?: throw IllegalArgumentException("Session $id not found")
    }

    override fun invalidate(id: String): Future<Unit> {
        sessions.remove(id)
        return CompletableFuture.completedFuture(Unit)
    }
}

Here is a minimal test that that shows how this behavior crashes the pipeline:
https://gist.github.com/audax/ac07afbe225d0d6a04f57a9fa492bffb

@cy6erGn0m cy6erGn0m self-assigned this Jan 25, 2017
@orangy orangy added the bug label Apr 24, 2017
cy6erGn0m pushed a commit that referenced this issue Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants