SC_Response::sendRedirect() transactionid= を画一的に追加せず、一定条件での継承のみとする #922

data/class/SC_Response.php

@@ -121,13 +121,13 @@
     /**
      * アプリケーション内でリダイレクトする
      *
-     * 内部で生成する URL の searchpart は、下記の順で上書きしていく。(後勝ち)
+     * 内部で生成する URL のクエリは、下記の順で上書きしていく。(後勝ち)
      * 1. 引数 $inheritQueryString が true の場合、$_SERVER['QUERY_STRING']
-     * 2. $location に含まれる searchpart
+     * 2. $location に含まれる クエリ
      * 3. 引数 $arrQueryString
      * @param  string    $location           「url-path」「現在のURLからのパス」「URL」のいずれか。「../」の解釈は行なわない。
-     * @param  array     $arrQueryString     URL に付加する searchpart
-     * @param  bool      $inheritQueryString 現在のリクエストの searchpart を継承するか
+     * @param  array     $arrQueryString     URL に付加するクエリ
+     * @param  bool      $inheritQueryString 現在のリクエストのクエリを継承するか
      * @param  bool|null $useSsl             true:HTTPSを強制, false:HTTPを強制, null:継承
      * @return void
      * @static
@@ -226,7 +226,22 @@
             $netUrl->addQueryString(session_name(), session_id());
         }
 
-        $netUrl->addQueryString(TRANSACTION_ID_NAME, SC_Helper_Session_Ex::getToken());
+        /**
+         * transactionid を受け取ったリクエストに関して、値を継承してリダイレクトする。
+         * @see https://github.com/EC-CUBE/ec-cube2/issues/922
+         */
+        if (// 管理機能 (本来遷移先で判定すべきだが、簡易的に遷移元で判定している。)
+            GC_Utils_Ex::isAdminFunction()
+            // 遷移元 transactionid 指定あり
+            && isset($_REQUEST[TRANSACTION_ID_NAME])
+            // リダイレクト先 mode 指定あり
+            && isset($netUrl->querystring['mode'])
+            // リダイレクト先 transactionid 指定なし
+            && !isset($netUrl->querystring[TRANSACTION_ID_NAME])
+        ) {
+            $netUrl->addQueryString(TRANSACTION_ID_NAME, $_REQUEST[TRANSACTION_ID_NAME]);
+        }
+
         $url = $netUrl->getURL();
 
         header("Location: $url");

tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php

@@ -0,0 +1,291 @@
+<?php
+
+class SC_ResponseSendRedirectWithHeaderTest extends Common_TestCase
+{
+    /** @var resource|bool */
+    private static $server;
+    const FIXTURES_DIR = '../fixtures/server';
+
+    public static function setUpBeforeClass()
+    {
+        $spec = [
+            1 => ['file', '/dev/null', 'w'],
+            2 => ['file', '/dev/null', 'w']
+        ];
+
+        if (!self::$server = @proc_open('exec php -S 127.0.0.1:8085', $spec, $pipes, __DIR__.'/'.self::FIXTURES_DIR)) {
+            self::markTestSkipped('PHP server unable to start.');
+        }
+        sleep(1);
+    }
+
+    public static function tearDownAfterClass()
+    {
+        if (is_resource(self::$server)) {
+            proc_terminate(self::$server);
+            proc_close(self::$server);
+        }
+    }
+
+    /**
+     * @param array $arrPostData
+     * @param array $arrTestHeader エスケープせず HTTP ヘッダーに埋め込むので注意。
+     * @param array|null $arrPostData
+     * @return void
+     */
+    private function request($arrQuery = [], $arrTestHeader = [], $arrPostData = null)
+    {
+        $netUrl = new Net_URL('http://127.0.0.1:8085/sc_response_sendRedirect.php');
+        $netUrl->querystring = $arrQuery;
+        $url = $netUrl->getUrl();
+
+        $arrOptions = [
+            'http' => [
+                'follow_location' => 0,
+                'header' => [],
+            ],
+        ];
+
+        if (isset($arrPostData)) {
+            $arrOptions['http']['method'] = 'POST';
+            $arrOptions['http']['header'][] = 'Content-Type: application/x-www-form-urlencoded';
+            $arrOptions['http']['content'] = http_build_query($arrPostData, '', '&');
+        }
+        foreach ($arrTestHeader as $key => $value) {
+            $arrOptions['http']['header'][] = "X-Test-{$key}: {$value}";
+        }
+
+        $contents = file_get_contents($url, false, stream_context_create($arrOptions));
+
+        return $contents;
+    }
+
+    /**
+     * @param array $arrQuerystring
+     * @return string
+     */
+    private function getExpectedContents($arrQuerystring = [])
+    {
+        $netUrl = new Net_URL('http://127.0.0.1:8085/redirect_url.php');
+        $netUrl->querystring = $arrQuerystring;
+        $url = $netUrl->getUrl();
+
+        $contents = file_get_contents(__DIR__ . '/' . self::FIXTURES_DIR . '/sc_response_sendRedirect.expected');
+        $contents = str_replace('{url}', $url, $contents);
+
+        return $contents;
+    }
+
+    /**
+     * 以下は、sendRedirect で transactionid が付加されないパターン。
+     */
+    public function testSendRedirect_Admin_GRG_transactionidなし_遷移先にmode()
+    {
+        $arrQuery = [
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'dst_mode' => 'hoge',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Admin_PRG_リクエストにtransactionid_modeなし()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+        ];
+        $arrPostData    = [
+            'foo' => 'bar',
+            TRANSACTION_ID_NAME => 'on_reqest_post',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader, $arrPostData);
+
+        $expected = $this->getExpectedContents();
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Front_GRG_リクエストにtransactionid_遷移先にmode()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'front',
+            'dst_mode' => 'hoge',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Front_PRG_リクエストにtransactionid_遷移先にmode()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'front',
+            'dst_mode' => 'hoge',
+        ];
+        $arrPostData    = [
+            'foo' => 'bar',
+            TRANSACTION_ID_NAME => 'on_reqest_post',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader, $arrPostData);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    /**
+     * 以下は、sendRedirect で リクエストの transactionid がリダイレクト先に引き継がれるパターン。
+     */
+    public function testSendRedirect_Admin_GRG_リクエストにtransactionid_遷移先にmode()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'dst_mode' => 'hoge',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Admin_PRG_リクエストにtransactionid_遷移先にmode()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'dst_mode' => 'hoge',
+        ];
+        $arrPostData    = [
+            'foo' => 'bar',
+            TRANSACTION_ID_NAME => 'on_reqest_post',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader, $arrPostData);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+            TRANSACTION_ID_NAME => 'on_reqest_post',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Admin_GRG_リクエストにtransactionid_modeなし_クエリ継承()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'inherit_query_string' => '1',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader);
+
+        $expected = $this->getExpectedContents([
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Admin_PRG_リクエストにtransactionid_modeなし_クエリ継承()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'inherit_query_string' => '1',
+        ];
+        $arrPostData    = [
+            'foo' => 'bar',
+            TRANSACTION_ID_NAME => 'on_reqest_post',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader, $arrPostData);
+
+        $expected = $this->getExpectedContents([
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    /**
+     * 以下は、sendRedirect で ロジックの transactionid がリダイレクト先に渡るパターン。
+     *
+     * 通常無さそうなケースだが、仕様として持っている動作。リダイレクトのタイミングで transactionid を更新する用途を想定。
+     */
+    public function testSendRedirect_Admin_GRG_ロジック・リクエストにtransactionid_遷移先にmode()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'dst_mode' => 'hoge',
+            'logic_transaction_id' => 'on_logic',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+            TRANSACTION_ID_NAME => 'on_logic',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testSendRedirect_Admin_PRG_ロジック・リクエストにtransactionid_遷移先にmode()
+    {
+        $arrQuery = [
+            TRANSACTION_ID_NAME => 'on_reqest_query',
+        ];
+        $arrTestHeader = [
+            'function' => 'admin',
+            'dst_mode' => 'hoge',
+            'logic_transaction_id' => 'on_logic',
+        ];
+        $arrPostData    = [
+            'foo' => 'bar',
+            TRANSACTION_ID_NAME => 'on_reqest_post',
+        ];
+        $actual = $this->request($arrQuery, $arrTestHeader, $arrPostData);
+
+        $expected = $this->getExpectedContents([
+            'mode' => 'hoge',
+            TRANSACTION_ID_NAME => 'on_logic',
+        ]);
+
+        self::assertSame($expected, $actual);
+    }
+}

tests/class/SC_Response/SC_ResponseWithHeaderTest.php

@@ -13,7 +13,7 @@ public static function setUpBeforeClass()
             2 => ['file', '/dev/null', 'w']
         ];
 
-        if (!self::$server = @proc_open('exec php -S 127.0.0.1:8053', $spec, $pipes, __DIR__.'/'.self::FIXTURES_DIR)) {
+        if (!self::$server = @proc_open('exec php -S 127.0.0.1:8085', $spec, $pipes, __DIR__.'/'.self::FIXTURES_DIR)) {
             self::markTestSkipped('PHP server unable to start.');
         }
         sleep(1);
@@ -36,7 +36,7 @@ public function testReload()
                 ]
             ]
         );
-        $actual = file_get_contents('http://127.0.0.1:8053/sc_response_reload.php', false, $context);
+        $actual = file_get_contents('http://127.0.0.1:8085/sc_response_reload.php', false, $context);
         self::assertStringEqualsFile(__DIR__.'/'.self::FIXTURES_DIR.'/sc_response_reload.expected', $actual);
     }
 }

tests/class/fixtures/server/common.php

@@ -2,7 +2,7 @@
 
 putenv('HTTP_URL=http://127.0.0.1:8085/');
 
-require __DIR__.'/../../../require.php';
+require __DIR__ . '/../../../require.php';
 
 error_reporting(-1);
 ini_set('display_errors', '1');

tests/class/fixtures/server/sc_response_reload.expected

@@ -2,6 +2,6 @@
 Array
 (
     [0] => Content-Type: text/plain; charset=utf-8
-    [1] => Location: http://127.0.0.1:8085/index.php?debug=%E3%83%86%E3%82%B9%E3%83%88&redirect=1&transactionid=aaaa
+    [1] => Location: http://127.0.0.1:8085/index.php?debug=%E3%83%86%E3%82%B9%E3%83%88&redirect=1
 )
 shutdown

tests/class/fixtures/server/sc_response_sendRedirect.expected

@@ -0,0 +1,7 @@
+
+Array
+(
+    [0] => Content-Type: text/plain; charset=utf-8
+    [1] => Location: {url}
+)
+shutdown