Skip to content

Instantly share code, notes, and snippets.

@hidenorigoto
Created May 8, 2011 00:45
Show Gist options
  • Save hidenorigoto/960995 to your computer and use it in GitHub Desktop.
Save hidenorigoto/960995 to your computer and use it in GitHub Desktop.
Request::hasSession()について
  • Request::hasSession()の現在の実装の意図

    1. (意図A)クライアントから送られてきた(本来の意味の)リクエストがセッション有効状態か(=Cookieを持っているか)を調べる
    2. (意図B)単にセッションオブジェクトがあるかないかという判定
    3. (意図C)セッションオブジェクトがあり、開始済みかどうか

    現在のhasSession()メソッドは、意図Aの実装であると考えられる。ただし、以下に挙げる呼び出し箇所では、意図Bなどが混在して用いられていると思われる。

  • Request::hasSession()の使われ方

    • a. RequestListener L70 上記の意図で判定に用いている。
    • b. RequestDataCollector L67 メソッドの意図と異なる使われ方と思われる。null !== $request->getSession() が適切ではないか。
    • c. SecurityComponent Http/Firewall/ContextListener L65 メソッドの意図と異なる使われ方と思われる。null !== $request->getSession() が適切ではないか。
    • d. SecurityComponent Http/Firewall/ExceptionListener L154 用いている意図がやや不明確。
@fivestar
Copy link

fivestar commented May 8, 2011

修正してみました。以前のブランチはいったん破棄して、新たに変更を行っています(そのためSession::isStarted()は実装していません)。

https://github.com/fivestar/symfony/commits/session
fivestar/symfony@ae0ef9e

合わせてExceptionListenerで行っていた箇所を、EntryPointへ移動しました。

@fivestar
Copy link

fivestar commented May 8, 2011

symfony/symfony#833

既に修正パッチがあがっているようでした。EntryPointへ移動した部分は、Johannes曰く、話し合いの結果ExceptionListenerから動かさないことを決めたとのことです。

あがっていたパッチではhasSession()はセッションオブジェクトの有無を確かめるメソッドに変更されており、CookieがあるかどうかをhasPreviousSession()として定義していました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment