Bug（バグ） #4055: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない - OpenPNE 3 - OpenPNE Issue Tracking System

Actions

Copy link

Bug（バグ） #4055

closed

OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない

Added by kaoru n about 8 years ago. Updated over 4 years ago.

Status:

Fixed（完了）

Priority:

Normal（通常）

Assignee:

Youichi Kimura

Target version:

OpenPNE 3.10.x

Start date:

2016-11-21

Due date:

% Done:

100%

Estimated time:

(Total: 0:00 h)

3.6 で発生するか:

No (いいえ)

3.8 で発生するか:

Yes (はい)

Description

Overview (現象)¶

OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターから日記等にアクセスする場合、http://example.com/sns/diary/1 のようなURLになるべきであるが、http://example.com/diary/1 にアクセスしてしまう。

Causes (原因)¶

通知センターに表示される通知のリンク先は、opNotificationCenter::notify() の url パラメーターによって指定される。
このパラメーターを実装した当初は diary/show?id=1 や @diary_show?id=1 のような url_for ヘルパーに指定するための Internal URI の形式を想定していた。
しかし、実際には誤った実装によって Internal URI での指定は正しく機能していない状態となっていた。
（opJsonApiHelper が実行されるのは api アプリケーションであるため、url_for ではなく app_url_for を呼び出すのが正しい）

source:apps/api/lib/helper/opJsonApiHelper.php@8205623#L203

    'url' => $notification['url'] ? url_for($notification['url'], array('abstract' => true)) : null,

通知センターによる通知機能を提供するプラグイン等では、上記の不具合を避けるために Internal URI ではなく /diary/1 などの URI を直接出力することで目的の URI へのリンクを実現している。

しかし、/diary/1 などの URI を指定した場合には、本来 url_for や app_url_for であれば自動的に付加されるサブディレクトリ部分の相対パス (sfWebRequest::getRelativeUrlRoot() に相当) が付加されないため、当チケットで報告されているような OpenPNE をサブディレクトリに設置した場合のリンクの不具合が生じる。

Way to fix (修正内容)¶

JSON API の /api.php/push/search.json の実装を修正し、opNotificationCenter::notify() の url パラメーターに Internal URI が指定された場合でも正しく動作するようにする。この場合は、API 実行時にサブディレクトリを考慮した URI が生成されるものとする。

また、/diary/1 のような URI を直接 url パラメーターに指定していたプラグイン等との互換性を保つため、先頭が / から始まる URI であった場合には app_url_for ヘルパーを介さずに直接文字列を出力するように対応する。
この場合は、API 側ではサブディレクトリ部分の付加は行わずに出力する。これは、opLikePlugin のようにプラグイン側で独自にサブディレクトリ部分を付加した URI を url パラメーターに指定している実装が存在しており、一律な対応を執ることが困難なためである。

Subtasks 3 (0 open — 3 closed)

Backport（バックポート） #4057: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない

Invalid（無効）

isao sano

2016-11-21

Actions

Backport（バックポート） #4058: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない

Fixed（完了）

Youichi Kimura

2016-11-21

Actions

Backport（バックポート） #4379: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない

Fixed（完了）

isao sano

2016-11-21

Actions

Related issues 1 (0 open — 1 closed)

Actions

Copy link

Updated by kaoru n about 8 years ago

Status changed from New（新規） to Accepted（着手）
Assignee set to kaoru n
Target version set to OpenPNE 3.9.0-old

Actions

Copy link

Updated by kaoru n about 8 years ago

Status changed from Accepted（着手） to Pending Review（レビュー待ち）
% Done changed from 0 to 50

https://github.com/openpne/OpenPNE3/pull/393
にてプルリクエストしました。
レビューをお願いします。

Actions

Copy link

Updated by isao sano about 8 years ago

Related to Backport（バックポート） #4057: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない added

Actions

Copy link

Updated by isao sano about 8 years ago

Related to Backport（バックポート） #4058: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスする日記やコミュニティのURLが正しくない added

Actions

Copy link

Updated by kaoru n about 8 years ago

Related to Enhancement（機能追加・改善） #4064: OpenPNE が http://example.com/sns/ のようにサブディレクトリ以下に設置されている場合に、通知センターからアクセスするURLが正しくない問題に対する opLikePlugin の独自修正を取り除く added

Actions

Copy link

Updated by Shinichi Urabe about 8 years ago

Status changed from Pending Review（レビュー待ち） to Rejected（差し戻し）

ソースを見る限り openpne.baseUrl には http://xxx.xxx/sns のように HTTP スキームから始まるような URL が入ることがないとは思われるが
含まれた場合 http:/xxx.xxx/sns のように置換されてしまう、念のためその点も考慮したほうがよいと思われる

var linkUrl = 'http://example.com//test/test/test//test';
linkUrl.replace(/\/\//g, "/");

→ 結果: "http:/example.com/test/test/test/test"

必要とされて // 連続するスラッシュがURLに含まれる可能性があるか分からないが、 data-location-url や openpne.baseUrl に含められる文字列を過剰に置換する可能性があるのは懸念点となる
対応としては結合される data-location-url の先頭、openpne.baseUrl の末尾に一つだけ / が存在するようにしておくほうが望ましい

Actions

Copy link

Updated by kaoru n about 8 years ago

Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）

https://github.com/openpne/OpenPNE3/blob/stable-3.8.x/apps/pc_frontend/templates/_layout.php#L22
のように、openpne.baseUrl は末尾に '/' が一つ付与されるため、data-location-url の先頭の '/' を取り除いて文字列を連結するようにプルリクエストを更新しました。
https://github.com/openpne/OpenPNE3/pull/393

Actions

Copy link

Updated by kaoru n about 8 years ago

3.6 で発生するか changed from Unknown (未調査) to No (いいえ)
3.8 で発生するか changed from Unknown (未調査) to Yes (はい)

Actions

Copy link

#10

Updated by Shinichi Urabe about 8 years ago

Status changed from Pending Review（レビュー待ち） to Rejected（差し戻し）

Javascript のコーディング規約はありませんが、少なくとも web/js/jquery.notify.js 内の文字リテラルは ' シングルクオートで囲んでいますので、統一しておいたほうがよいです
dataLocationUrl.replace(/^\/*/g, ""); の空文字のダブルクオートの箇所

Actions

Copy link

#11

Updated by kaoru n about 8 years ago

Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）

変換文字列をダブルクォートではなくシングルクオートで囲むように修正しました
https://github.com/openpne/OpenPNE3/pull/393

Actions

Copy link

#12

Updated by Shinichi Urabe about 8 years ago

Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
% Done changed from 50 to 70

Actions

Copy link

#13

Updated by kaoru n about 8 years ago

Status changed from Pending Testing（テスト待ち） to Rejected（差し戻し）
% Done changed from 70 to 50

各プラグインにてサブディレクトリについて考慮されているものもあるため、下記の方針での修正とします。
・渡されたURLが「/」からはじまっている場合は、通知センター側でそのまま使用する、インターナルURIで渡された場合は、サブディレクトリ考慮するよう対応する
・プラグイン側は、現在サブディレクトリを考慮している場合はそのままでよい、考慮してないない場合は、インターナルURIで渡すように変更する

Actions

Copy link

#14

Updated by kaoru n about 8 years ago

Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）

#4055-13 の仕様となるよう
https://github.com/openpne/OpenPNE3/pull/393
を更新しました。

渡されたURLが「/」からはじまっている場合のうち、「///」とスラッシュが連続していても通知センターでは考慮しません。
また、「http://」等から始まっている場合でも、「/」から始まっていないため、openpne.baseUrl を先頭に連結します。

Actions

Copy link

#15

Updated by kaoru n about 8 years ago

各プラグインの状況

opLikePlugin 独自にサブディレクトリを考慮している( #4064-8 参照)
opCommunityTopicPlugin(1.1.x) 独自にサブディレクトリを考慮している
opMessagePlugin(2.0.0) 独自にサブディレクトリを考慮している
opTimelinePlugin
https://github.com/tejimaya/opTimelinePlugin/blob/master/lib/util/opTimelinePluginUtil.class.php#L36
にて、通知するよう実装があるが、hasScreenName() はどこからもコールされていないので、現状通知は行われない
対応するチケットのみ作成 #4075

Actions

Copy link

#16

Updated by Shinichi Urabe about 8 years ago

Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
% Done changed from 50 to 70

Actions

Copy link

#17

Updated by Youichi Kimura about 8 years ago

Status changed from Pending Testing（テスト待ち） to Pending Review（レビュー待ち）
% Done changed from 70 to 50

#4055-13 についてですが、通知の URL に internal uri (@obj_member_profile?id=1 や member/1 のような形式) が使用された場合にサブディレクトリを考慮した URL を出力するというのは、JS 側ではなく API 側での対応を想定していました。
// 伝わりにくくてすみません...

以下の Pull Request が想定している修正内容です。
https://github.com/openpne/OpenPNE3/pull/433

各プラグインにおける修正との兼ね合いもあるため、この内容でプラグインが対応可能であるかもどうか念のため確認をお願いします。

Actions

Copy link

#18