diff --git a/data/class/SC_Response.php b/data/class/SC_Response.php index 018858e718..09c9531076 100644 --- a/data/class/SC_Response.php +++ b/data/class/SC_Response.php @@ -121,13 +121,13 @@ public static function actionExit() /** * アプリケーション内でリダイレクトする * - * 内部で生成する 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 @@ public static function sendRedirect($location, $arrQueryString = array(), $inher $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"); diff --git a/tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php b/tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php new file mode 100644 index 0000000000..680aacfbab --- /dev/null +++ b/tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php @@ -0,0 +1,291 @@ + ['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); + } +} diff --git a/tests/class/SC_Response/SC_ResponseWithHeaderTest.php b/tests/class/SC_Response/SC_ResponseWithHeaderTest.php index 56299bb2d7..55cd0a4713 100644 --- a/tests/class/SC_Response/SC_ResponseWithHeaderTest.php +++ b/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); } } diff --git a/tests/class/fixtures/server/common.php b/tests/class/fixtures/server/common.php index ca9ccba031..9d675a8d79 100644 --- a/tests/class/fixtures/server/common.php +++ b/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'); diff --git a/tests/class/fixtures/server/sc_response_reload.expected b/tests/class/fixtures/server/sc_response_reload.expected index 483664dd24..46892f56a0 100644 --- a/tests/class/fixtures/server/sc_response_reload.expected +++ b/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 diff --git a/tests/class/fixtures/server/sc_response_sendRedirect.expected b/tests/class/fixtures/server/sc_response_sendRedirect.expected new file mode 100644 index 0000000000..a828def87c --- /dev/null +++ b/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 diff --git a/tests/class/fixtures/server/sc_response_sendRedirect.php b/tests/class/fixtures/server/sc_response_sendRedirect.php new file mode 100644 index 0000000000..e1f106c689 --- /dev/null +++ b/tests/class/fixtures/server/sc_response_sendRedirect.php @@ -0,0 +1,33 @@ += 1) { + $url .= '?mode=' . $arrHeader['X-Test-dst_mode']; +} + +if (strlen($arrHeader['X-Test-logic_transaction_id'] ?? '') >= 1) { + $arrQueryString[TRANSACTION_ID_NAME] = $arrHeader['X-Test-logic_transaction_id']; +} + +$inherit_query_string = ($arrHeader['X-Test-inherit_query_string'] ?? '') === '1'; + +SC_Response_Ex::sendRedirect($url, $arrQueryString, $inherit_query_string);