From 4d07dc44cec3571d61348473bde5bcb2c2e0d442 Mon Sep 17 00:00:00 2001 From: Seasoft Date: Fri, 26 Jul 2024 16:20:23 +0900 Subject: [PATCH] =?UTF-8?q?iSC=5FResponse::sendRedirect()=20transactionid?= =?UTF-8?q?=3D=20=E3=82=92=E7=94=BB=E4=B8=80=E7=9A=84=E3=81=AB=E8=BF=BD?= =?UTF-8?q?=E5=8A=A0=E3=81=9B=E3=81=9A=E3=80=81=E4=B8=80=E5=AE=9A=E6=9D=A1?= =?UTF-8?q?=E4=BB=B6=E3=81=A7=E3=81=AE=E7=B6=99=E6=89=BF=E3=81=AE=E3=81=BF?= =?UTF-8?q?=E3=81=A8=E3=81=99=E3=82=8B=20#922?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- data/class/SC_Response.php | 18 +- .../SC_ResponseSendRedirectWithHeaderTest.php | 291 ++++++++++++++++++ .../SC_Response/SC_ResponseWithHeaderTest.php | 71 +---- tests/class/fixtures/server/common.php | 2 +- .../server/sc_response_reload.expected | 2 +- .../fixtures/server/sc_response_reload.php | 7 +- .../sc_response_reload_add_transactionid.php | 11 - .../server/sc_response_sendRedirect.expected | 7 + .../server/sc_response_sendRedirect.php | 33 ++ 9 files changed, 353 insertions(+), 89 deletions(-) create mode 100644 tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php delete mode 100644 tests/class/fixtures/server/sc_response_reload_add_transactionid.php create mode 100644 tests/class/fixtures/server/sc_response_sendRedirect.expected create mode 100644 tests/class/fixtures/server/sc_response_sendRedirect.php diff --git a/data/class/SC_Response.php b/data/class/SC_Response.php index f9890a3f88..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 @@ -230,7 +230,15 @@ public static function sendRedirect($location, $arrQueryString = array(), $inher * transactionid を受け取ったリクエストに関して、値を継承してリダイレクトする。 * @see https://github.com/EC-CUBE/ec-cube2/issues/922 */ - if (isset($_REQUEST[TRANSACTION_ID_NAME]) && !isset($netUrl->querystring[TRANSACTION_ID_NAME])) { + 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]); } diff --git a/tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php b/tests/class/SC_Response/SC_ResponseSendRedirectWithHeaderTest.php new file mode 100644 index 0000000000..028405e783 --- /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:8053', $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:8053/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:8053/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 5b8a9fb766..56299bb2d7 100644 --- a/tests/class/SC_Response/SC_ResponseWithHeaderTest.php +++ b/tests/class/SC_Response/SC_ResponseWithHeaderTest.php @@ -27,77 +27,16 @@ public static function tearDownAfterClass() } } - private function file_get_contents($url) + public function testReload() { $context = stream_context_create( [ 'http' => [ - 'follow_location' => 0, - ], + 'follow_location' => false + ] ] ); - - $contents = file_get_contents($url, false, $context); - - return $contents; - } - - private function getExpectedContents($url, $additional_query_strings = '') - { - $contents = file_get_contents(__DIR__ . '/' . self::FIXTURES_DIR . '/sc_response_reload.expected'); - - $url .= ''; - - if (strlen($additional_query_strings) >= 1) { - $url .= '&' . $additional_query_strings; - } - - $contents = str_replace('{url}', $url, $contents); - - return $contents; - } - - public function testReload_transactionidが絡まない() - { - $request_url = 'http://127.0.0.1:8053/sc_response_reload.php?debug=' . urlencode('テスト'); - $expected_url = $request_url . '&redirect=1'; - $expected = $this->getExpectedContents($expected_url); - - $actual = $this->file_get_contents($request_url); - self::assertSame($expected, $actual); - } - - public function testReload_リクエストにtransactionidを含む() - { - $request_url = 'http://127.0.0.1:8053/sc_response_reload.php?debug=' . urlencode('テスト') . '&' . TRANSACTION_ID_NAME . '=on_reqest'; - $expected_url = $request_url . '&redirect=1'; - $expected = $this->getExpectedContents($expected_url); - - $actual = $this->file_get_contents($request_url); - self::assertSame($expected, $actual); - } - - public function testReload_ロジックにtransactionidを含む() - { - $request_url = 'http://127.0.0.1:8053/sc_response_reload_add_transactionid.php?debug=' . urlencode('テスト'); - $expected_url = $request_url . '&redirect=1&' . TRANSACTION_ID_NAME . '=on_logic'; - $expected = $this->getExpectedContents($expected_url); - - $actual = $this->file_get_contents($request_url); - self::assertSame($expected, $actual); - } - - public function testReload_ロジック・リクエストにtransactionidを含む() - { - $base_url = 'http://127.0.0.1:8053/sc_response_reload_add_transactionid.php?debug=' . urlencode('テスト'); - $request_url = $base_url; - $request_url .= '&' . TRANSACTION_ID_NAME . '=on_reqest'; - $expected_url = $base_url; - $expected_url .= '&' . TRANSACTION_ID_NAME . '=on_logic'; - $expected_url .= '&redirect=1'; - $expected = $this->getExpectedContents($expected_url); - - $actual = $this->file_get_contents($request_url); - self::assertSame($expected, $actual); + $actual = file_get_contents('http://127.0.0.1:8053/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 28d5837fd2..842a90bd6b 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:8053/'); -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 a828def87c..ce209a8e69 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: {url} + [1] => Location: http://127.0.0.1:8053/index.php?debug=%E3%83%86%E3%82%B9%E3%83%88&redirect=1 ) shutdown diff --git a/tests/class/fixtures/server/sc_response_reload.php b/tests/class/fixtures/server/sc_response_reload.php index 0ed10702c6..63e8df8511 100644 --- a/tests/class/fixtures/server/sc_response_reload.php +++ b/tests/class/fixtures/server/sc_response_reload.php @@ -2,10 +2,7 @@ require __DIR__.'/common.php'; -/** - * この値は使われない。 - * @see https://github.com/EC-CUBE/ec-cube2/issues/922 - */ -$_SESSION[TRANSACTION_ID_NAME] = 'on_session'; +$_SERVER['REQUEST_URI'] = HTTPS_URL.'index.php?debug='.urlencode('テスト'); +$_SESSION[TRANSACTION_ID_NAME] = 'aaaa'; SC_Response_Ex::reload(['redirect' => 1]); diff --git a/tests/class/fixtures/server/sc_response_reload_add_transactionid.php b/tests/class/fixtures/server/sc_response_reload_add_transactionid.php deleted file mode 100644 index 1fba3ea491..0000000000 --- a/tests/class/fixtures/server/sc_response_reload_add_transactionid.php +++ /dev/null @@ -1,11 +0,0 @@ - 1, TRANSACTION_ID_NAME => 'on_logic']); 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..b6dbbcbcf4 --- /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);