現在/favicon.icoがないことで、正常なアクセスであってもブラウザから複数回の無駄なアクセスが発生しています。

存在しないfaviconへのアクセスは（404的な挙動で）/にリダイレクトされ、そこからさらに/{なんらかのブログ}/にリダイレクトされています。

提案

• faviconを作成し、追加する
• 404相当のルートを作成する
• .htaccessに記述してApacheで404を返す

上から順によいと思います、いかがでしょうか。ご検討ください。

https://github.com/fc2blog/blog/blob/8dc48927e88656331d36d1fbb11be302c192954e/app/config/fc2_template.php

には多数のテンプレート用のタグ定義がありますが、基本のテンプレートでは使われているタグが少なく、タグが正常動作するのかテストできておりません。

（できるかぎり）すべてのタグを埋め込んだテンプレートがテストに必要かと思います。

課題

「正しさ」を最初にどのように定義するのか見通しがない。（エラーが出ない、はわかるが）

現状のコードは歴史由来とおもわれる課題があると感じており、初見だと見通しが悪く、実際にUnitTestや静的解析ツールとの相性がよくありません。

• namespace無し
• 同名クラスの存在（CommonControllerや、BlogsControllerなど）
• require, require_onceによる手動クラスロード （require_once(Config::get('CONTROLLER_DIR') . 'user/user_controller.php');など）
• 独自のファイル名規則、ファイル配置規則 （キャメルケース、フラット構造）
• テンプレートファイル拡張子が.html（実質的にphp）

これらは昨今の統一されつつあるPHPのスタイルと乖離があります。
今後も機能は増えていくとおもいますので、早めに改善をおこなうことを提案します。

現状の考察

現状のファイル数は以下の通りで、まだ規模としては小さいと言えますので、作業自体には長い期間はかからない見通しです。

$ find app |grep php |wc -l
      83
$ find app |grep html |wc -l
     114

具体的な改修内容

• namespaceの適用
• クラス系ファイルの再配置
• クラスローダをComposerに
• テンプレートファイルをhtmlからphpへ

namespaceについては、\FC2\Blogを基底として、\FC2\Blog\Web\Controller\User\BlogsController、\FC2\Blog\Model\BlogsModel、などといった命名規則を想定していますが、もし社内版とのクラスと衝突がありそうであれば指摘ください。

なお、この提案では一旦速やかに完了させ、なおかつできるだけ壊さないために、以下は当座含みません。

• コードで、クラス名以外のメソッド名、関数名の是正（いわゆるPSR-1的な命名規則の適用）
• 型定義の付与など
• コードフォーマット
• 根本的なブートストラップ構造の変更
• クラスファイル以外のディレクトリ構造の変更
• ルーターの根本的な設計変更（namespace対応のため、$argscへprefixを差し込むなどの処理は入れる予定）
• cron.phpやconfig系の構造変更
• 「クラス化されていない関数」（例：getFc2PagingPHP等）のクラス化

懸念など

現在安全なPHP開発を行うには静的解析ツールの支援は不可欠といってよいと考えており、早めに手をつけるべきだと考えております。しかしながら、既存システムがどうしてもこの形でなければならないなどあれば、こういったリファクタは不可能などあるかもしれません、ご検討ください。

mt_rand等の差し替え

• https://github.com/fc2blog/blog/blob/master/app/lib/CaptchaImage.php#L16
• https://github.com/fc2blog/blog/blob/master/app/core/app.php#L387

等においてmt_rand()等が利用されているが、mt_rand等は暗号用途等に利用してはならない(ref, ref2)ため、よりよいものに代替する。

md5+時刻によるランダム文字列生成の差し替え

• https://github.com/fc2blog/blog/blob/master/app/controller/app_controller.php#L68
• https://github.com/fc2blog/blog/blob/master/app/view/admin/common/install.html#L172

一部にtime()とmd5()を用いたtoken生成があり、推測が可能なためにこれをより良いものに変更する。
（そもそもtimeを用いる必要がなさそう）

TBD

• コード上、実質タブレットと携帯（ガラケー）はほとんどPCになっている。
• 意味がないコードであれば、削除して、PC/SPに揃えたい。

READMEの以下はMITライセンスと適合しないと思われます。

*Do not remove the link to FC2 in the template.

MITライセンスより抜粋

〜without limitation the rights to use, copy, modify, 〜

前提

以下のフッター表記のことだとかんがえておりますが、別のところを指しているとしたらご指摘ください。

解決手法

1. 上記の「修正に対する制限」の記述を削除する
2. 「特別な保護をしたいテンプレート」を分離し、そちらにはそのような独自の（すくなくともMITなどではない）ライセンスを適用する。

テンプレートを独立させる場合は、利便性を考えるとインストール時に自動的に展開されるようにするなどの実装が必要。
しかしなんらか"Generic" なテンプレートを添付しないと「MITで成り立つBlogシステム」なのか疑問がのこる。

ref: #28

で「手軽にE2Eが利用できた方が良いと思いますので、ご提案の形で今後拡充を進めていただきたいです。」というところで、実際のテストを実装したいとおもいます。

まずは(管理画面以外の) ブログトップ、各種エントリなどの周回アクセスし、エラーなどが表示されないかを自動化テストするとよいと考えています。

• #28 をベースに周回テストを作成
• 周回URLをどのように考えるか検討しつつ実装
• コメント機能にCaptchaがあるので、そのバックドアを作成？実装方法は検討
• 全パターンを網羅するのは難しそうなので（しらないケースがあるかもしれないので）わかる範囲でとりあえず実装し、今後拡充

【原因】

コメント投稿時に生成・登録されるパスワードのハッシュ値がDBのサイズを超える値を登録しようとし、サイズを超えた値が切り捨てられた登録されています。
そのためコメント編集時に入力されたパスワードをハッシュ化した値とDBに登録した値の比較・チェックに失敗しています。

【例】

0dd970a5dbdc4809a2b3897ae9dd1576bc9ac2788688c33d09799c8c636e03c1　←コメント編集時に入力・ハッシュ化された値
0dd970a5dbdc4809a2b3897ae9dd1576bc9ac2788688c33d09　←コメント投稿時にDBに登録・ハッシュ化された値

【処理箇所】

• DB
  commentsテーブル->password
• ハッシュ化処理
  app/model/comments_model.php -> passwordHash()
• パスワードの比較処理
  app/model/comments_model.php -> password_check()

【補足】

他のパスワード機能が正常に動作しているのは、文字コードにutf8mb4の指定がないためです。
commentsテーブル->passwordにだけutf8mb4の指定がされているため、1文字に使用する最大バイト数が異なります。

【事象】

フロントのフロント詳細画面でコメントの改行箇所に
が表示され、改行表示されていません。

【原因】

テンプレートファイルでコメントをHTMLエンティティに変換しているため。
<div class="sub_body"><?php if(isset($comment['body'])) echo h($comment['body']); ?></div>
※h関数はhtmlentities関数を呼び出しています。

現状、ifや正規表現を用いたルーティングを行っているが、分かりづらいので一般的なルーター(nikic/fastrouteなど)への置換を行い、宣言的にしたい。

課題

現在のコードは（特に）管理画面においてStrutsスタイルの「クラス」「アクション」をURLから取る設計になっているが、パターンによってDenyする格好となっており、存在するすべてのRouteが把握できないことや、本当に危険なパターンがDenyできているかが怪しいです。Denyリスト方式より、Allowリスト方式のほうがセキュリティ的にも望ましいといえます。

デメリット

デメリットとして、今後はクラスやアクション追加時にルート定義をなんらか追加する必要があるが、大した手間ではないかと思われる。

現在、一部のルートはクエリパラメーターmode=Class&process=Actionのような形式になっているが、/mode/actionのような一般的な形式に一部書き換える必要がある。（そうしないことは技術的に可能ではあるが、そうしないと、宣言的にならないため）

つまり、URLが変わる。ただ、基本的にそのようなものを多用しているのは管理画面なので、ブログとしてのパーマリンクを壊すことはないのではないかとおもわれる。

There is no license file, nor license notation.

But this program uses GPL library below:
https://github.com/fc2blog/blog/blob/master/app/lib/SimpleImage.php#L9

So it should be GPL.

【事象】

初回インストール後に公式プラグインのDBテーブルの全レコードの更新日時（pluginsテーブル->updated_at）に「0000-00-00 00:00:00」が登録されている
※システムの機能上、影響が出ている箇所はない。

【原因】

DB登録に使用する更新日時の変数名が間違っていたので、カラム指定がされていなかったため。
※MySQL5.6以降の環境ではエラーとなりインサート自体できません。

【改修】

下記プルリクエストにて改修済み。
#4

ref #83 より

@uzulla
レスポンシブデザインのテンプレートも増えてきていることから、
PC・スマホの区別も不要になりつつある状況ですが、
PC・スマホのくくりだけ維持をしておき、それ以外は撤廃としていただいて問題ありません。

• テンプレート周り
• Config周り

の修正

• Device IDは当座そのまま（振り直しなどは当座しない）

【事象】

1. 記事一覧の各行のチェックボックスを選択
2. 「選択したものを全て削除する」ボタン押下
3. 記事一覧へ遷移（記事は１件も削除されていない）

【原因】

POST先URLが admin/entries/index になっているため。
※リクエストパラメータに下記が含まれていますが、管理画面のルーティング処理で下記は使用されていないように見受けられます。

'mode' => 'entries',
'prosess' => 'delete'

#17 より

概要

ブログ設定にて

• 「SSL設定」の「有効」「無効」スイッチを設置する
• 有効だと
  • 内部的な該当ページへのリンクのSchemaをhttp、またはhttpsに設定する
  • 設定と異なるschemaでアクセスされた場合、リダイレクトする
    • https->httpもリダイレクトする
  • ステータスコード指定ができるとなお望ましい
• この設定はブログID毎に指定できる

スイッチ差し込み場所

note

テンプレートに記載されているアセットなどのリンクURLにおけるschemaは特に変更されない。
あくまでも同一ブログシステム内での内部リンクでのみ作用する。

/admin/users/loginにて

「管理画面へログイン」などと表示すべきでしょうか？

SSL環境下でアプリが正しく動作するか未確認なため。

• 調査（方法から見当）
  • HTTPSで動作するdocker環境構築
  • なんらか一連の確認ケース・シナリオの策定
  • テスト
    • 可能であれば自動テストの作成
• 修正内容の確定
• 修正作業

検討事項

• jqueryなどを同梱するか？（CDNからのままにするか？） → fc2.comドメイン以外は同梱
• assetsを同梱するか？ → 同梱しない

【事象】

1.タグ一覧の各行のチェックボックスを選択
2.「選択したものを全て削除する」ボタン押下
3.タグ一覧へ遷移（記事は１件も削除されていない）

【原因】

POST先URLが admin/tags/index になっているため。
※リクエストパラメータに下記が含まれていますが、管理画面のルーティング処理で下記は使用されていないように見受けられます。

'mode' => 'tags',
'prosess' => 'delete'

より参照されている、

https://github.com/fc2blog/blog/blob/2c92bebecf766ad4b76eaf05c1d0af7c097062b6/public/js/template_comment.js

ファイルが空ファイル？の模様。

意味がないのであれば削除とおもいますが、なにか意味があるファイルでしょうか？
（テンプレート変換のタグは残すにせよ）

/admin/install.php のインストール後のメッセージに「またadmin/install.phpは不要ですので削除してください」と表示されますが、お願いは多くの場合してもらえませんし、削除したほうがよければ削除するロジックをいれてもよいかと思います。

また、「インストール後に再度操作してもなにもおこらない」のはコードをよむとわかりますが、画面上は「再インストール（初期化）」ができるように見えるので、そこも多少錯誤を生むかなと思われます。
（実際には二度三度と実行しても、不活性化（チェックをして、問題がなければなにもしない）されています）

しかしながら、DB接続情報が表示されてしまうのは少々問題かと思います。

対応策の検討

将来的に自動アップデートを入れることを考えると、配布ファイルが固定されない（減ったりする）のは不便かなと考えております。

すでに「インストール済みかどうか」を確認するロジックは存在しているので、アクセス時に「インストール済みです」と表示してしまうように挙動を変更するのはいかがでしょうか。

ご検討いただければ幸いです。

対応策

こちらにて方針を頂いた
ロックファイルを実装する

【事象１】

事象：「選択したものを全て削除する」ボタンが押下可能な状態にならない
原因：PHP出力値によりJavaScriptが構文エラーとなるため

【事象２】

事象：削除時にエラーとなる
原因2：PHP出力値によりJavaScriptで未定義の変数のエラーになるため

【改修】

下記プルリクエストにて改修済み。
#4

【事象】

1. コメントの各行のチェックボックスを選択
2. 「選択したものを全て削除する」ボタン押下
3. コメント一覧へ遷移（コメントは１件も削除されていない）

【原因】

POST先URLが admin/comments/index になっているため。
※リクエストパラメータに下記が含まれていますが、管理画面のルーティング処理で下記は使用されていないように見受けられます。

'mode' => 'comments',
'prosess' => 'delete'

現状のコードのCookie周りの仕様について、意見があります。
いまはかなり古い設計となっており、もっとモダン（セキュア）な設定にすべきかと思われます。

• cookieのdomainが指定されている
  • cookieのドメインは「指定しなければ」現在のホストになるので、十分です。
  • cookieのドメインが間違っていると、セッションが付与できません（そして、気づきづらい=>設置者が困る）
  • ワイルドカード的な活用をする「場合にのみ」、指定する意義はあるかと思いますが、基本は不要で必要な時に設定できる形が良いかと思います
• cookieにhttponly属性が付与されていません、つけるべきだと思います。（JSから触る必要な箇所がみつけられませんでした）
• cookieに明示的なsamesite指定が有りません、つけるべきだと思います。（いずれにせよ今後Laxが最低限になるので）
• https環境ならsecureを指定するべきです（httpsであるかどのように検知するかは議論の余地があります）
  • http,httpsデュアルスタックのサイトであったとしても、管理画面はhttps固定にするなどが必要ではないかと思います。（これは別のISSUEになりますが、公開URLがhttp・https両対応は良いとしても、管理画面はhttpsが利用可能なら必須とすべきと思います。）

現状のCookie名（キー名）設計について

DOMAIN設定からひっぱる形で、
SESSION_DEFAULT_DOMAIN
COOKIE_DEFAULT_DOMAIN
と2つにわかれているのもちょっとよくわかっておりません。
（過去の設計がこれを必要としていたのかもしれませんが、その理由が暗黙なので、普通にコードを読むと統一すべきように見えます）

実際、コード上でも統一されておらず、一部ではドメインを指定していないコードがあります。

具体的な改修案

• domainをデフォルトでは外す
• samesite=lax指定する（できるなら、必要がでるまではstrict。Noneは必要がないなら避けるべきですし、https必須になるので）
• httponlyを付与する(JSでセッションのCookie値を参照する箇所はないかと思っております）

note

// SESSIONのID名
$config['SESSION_NAME'] = 'dojima';

単なる好奇心(?)なのですが、dojimaとはなんでしょう？

#58 にて「（faviconは）商標が含まれるから本レポジトリに同梱不可」ということで、気になって確認したのですが、

https://github.com/fc2blog/blog/blob/master/public/img/admin/admin_icon2_02.png
https://github.com/fc2blog/blog/blob/master/public/img/admin/admin_icon3.png
https://github.com/fc2blog/blog/blob/master/public/img/admin/sp/sp_blog_detail_bg.png
https://github.com/fc2blog/blog/blob/master/public/img/admin/sp/[email protected]

これらの画像に含まれているFC2ロゴのファイルは大丈夫でしょうか？

テストを増やしてほしいというご依頼から、現在は管理画面のテストを増やすために各種調査や検討をおこなっておりますが、いくつか課題があると感じております。

現状

まず、現在の設計においてコントローラーは以下のように推移します。

app/src/Web/Controller/Admin/UsersController.phpにおいて

• （初期化）
• beforeFilter() → シングルトンから情報をとりだして認証などを実行したり、シングルトン内のデータを加工など
• ${ルータで解決された}method(); → コントローラ実行、返り値はテンプレートファイル名のみ、データはシングルトンから取り出し、保存
• afterFilter() → 現在は空、将来用？
• layout($template) → 内部でextract()をつかって、一方向なレンダリング（これは別でIssueを立てます）

この設計はユニットテストがしやすくありません。

（ユニットテストができないとE2Eテストにならざるをえませんが、E2Eテストは時間がかかり、なおかつ確認するのはHTML解釈から行う必要があるので、工数コストが高すぎます）

状態が複数あり（Request、Config、Controller->data）、シングルトンの多用がきびしい

現状、Filterやmethod、レンダラそれぞれでシングルトン（≒グローバル変数）のRequestやConfigを内部で読み書きしたり、コントローラの$this->dataの読み書きする設計となっております。（テンプレートから、直でConfigをコールしたりもしている）

// 例、Admin/BlogsControllerより、引数でなく、内部でシングルトンから読み込み
  public function create()
  {
    $request = Request::getInstance(); 

これは

• Configという、一見不変に見えるものにも状態が持たれている
  • （個人的には、Configは便利配列をめざしすぎており、構造体的なクラスに変更したほうが良いとおもいますが、それは一旦さておき）
• 複数箇所でデータの受け渡しがされている、どこを監視すべきかわかりづらい
• 初期化しづらい（RequestやConfigはシングルトン設計になっている）

という所でユニットテスト作成がしづらい状態です。

テストのないPHPアプリでは問題になりませんが（正直、シングルトンは実装においては楽で、メリット大だったのだろうなと思います）、ユニットテストでは同一の実行中で多数の仮想的なリクエストを処理できることが望ましく、状態を安全にリセットできないと厳しかったり、リクエストが明示であったり、複製できたほうが便利です。

よって、テストのためには、メソッドやフィルタは状態を渡して状態を返してもらうように（グローバル変数的なものを避ける）再設計すべきかと思います。

• Requestはシングルトンをやめて、都度生成、引数渡しにする
• Configで「状態」を持つパラメタは、ConfigでなくRequestに移譲する
  • 処理中になんらか汚染されているか判定が難しいので（cloneしておくことも考えられますが、スマートではない）
  • Configは、アプリ起動時に一度だけ設定されるような値だけを入れるなど
• 各メソッド・フィルタは状態を返し、渡していく（Requestを加工していく、等）
  • 「テンプレートファイル名」も「状態」に含むべきと思います
• redirectなどの早期exitを廃止（現在擬似Exit例外で回避していますが、これもテスト対応のためにやめるべきでしょう）

現状の設計のままテストをするなら

色々なところにユニットテスト中か確認するIFをつくり、「抜き取り」「リセット」をするためのスイッチをつける（現在、取り急ぎそのようにしていますが、スマートといえない））ことになるかと思います。

ただ、そのようにすると、事情に詳しくないと「テストが書けない」「状態をどこに追加すべきか分かりづらい」ので、好ましくはないかなと思います。

テストの書きやすさを上げないかぎり、開発者が気軽にテストを書かなくなります（私もきびしい）、そこの解決は早めにできればなとおもわれます、ご検討いただければ幸いです。

ref #36 より

概要

• app/view以下のhtmlファイル群はphpではあるが拡張子がhtmlである
• これらのファイルはhtmlとして外部から参照されることはなく、PHPとして解釈（incluide)される
• 静的解析ツールなどの利便性を考えると、php 拡張子にしたほうが良いと思われる

目標

• AMIイメージを作成し、手軽にAWSEC2インスタンスとしてFc2blogを試せるようにする

概要

• (AWSのコンソールでEC2を独力で立てられる程度のスキルを前提)
  • たとえばセキュリティグループの設定など責務外
• 各種クレデンシャルは生成、必要ならsshログインして参照させたり、AWSコンソールなどから参照させる
• sshキーはAWSに登録しているものが自動的にインスタンスに設定されるのでOK
• ディストリビューションはUbuntu 20
• ミドルウェアはアップデートの都合からディストリビューションの標準を活用
• インスタンス内にLAMPを構築
  • RDSなどマネージドは使わない（スケールアウトも無し）
  • （バックアップしたいのであれば、スナップショットをつかってもらう）

課題

• httpsをどうするか？（httpのみでよいか？）
  • ドメインが確定していれば、Let'sEncryptまで組めるが
  • httpsが必要なら、CloudFrontでやってもらう（SSL ターミネーションする）？

工程

• AMIベースイメージ決定
• 半自動化されたセットアップ手段作成
  • ミドルウェアインストール・更新
  • ミドルウェア設定
  • fc2blog アプリデプロイ
  • 動作チェック
  • クレデンシャル初期化スクリプト（AMI作成直前の掃除）作成
  • 公開AMIにするため、cloud-initによるクレデンシャル生成など調整
• 一式をベースに手順書を作成
• (FC2様で)インスタンスを作成、上記を実行し、AMIを保存、公開する

- testblog2となっているが記事一覧 - testblog2などとなるのがただしいのではないか？

事象

初期テンプレートやダウンロード可能なテンプレートに使用されているテンプレート用変数の一部が置換されず、そのまま出力されている。(100件以上)

原因・経緯

テンプレートはFC2ブログ側でも使用し変数が追加実装されていったが、OSS版では開発がされていなかったため機能実装に差異が生じてしまったため。

対応方法

OSS版対応で対応できる変数は置換・出力されるように実装し、OSS版に実装されていない機能の変数は空文字で出力されるよう改修を行う。

テンプレート用変数の仕様

【事象】

ブログ詳細画面にてパスワード保護のブログに正しいパスワードを入力しても同じ画面が再描画され閲覧できません。
トップページからパスワードを入力した場合は閲覧可能でした。

【原因】

$requestのキー'process'の値が'password'になっていなければいけませんが、下記で'view'に切り替えているためパスワードの認証処理(app/controller/user/entries_controller.php->password)に入っていません。

app/config/routing_user.php：27行目

26:if (strpos($query, 'no=')===0) {
27:  $request->set($argsa, 'view'); 
28:  $request->set('id', $request->get('no'));
29:}

※トップページで正常にパスワード認証されるのはURLパラメータに'no='が含まれていないので、上記処理に入らないため。

作業

• リストアップ＆調査
• テストコード
• 変換

#78 (comment)
に記載したが、

現在のテンプレート用のタグ（のPHPコード）は、その内で子孫タグが用いるデータを生成しており、変数スコープがフラットである前提の設計になっている。
そのために、現在想定される手法ではヘルパー関数などへの切り出しが困難（extract,compactを多用すれば解決できるが、良い解決策に見えない）なので、いったんテスト制作でヘルパータグの品質担保までとする。

以下は当初の目的であったが、完遂しなかった。

現在、タグを置換するために大量にタグとPHP生コードを置換するコードがfc2_template.phpにあります。

上記のような「ロジック」を置換するPHPコードをベタ書きするのは見通しが悪く、あまり安全にみえません。

可能ならば、そういったロジックをリストアップし、Wrapするヘルパー関数化して見通しをよくすることを提案いたします。

(現代のPHPは十分に高速なので、関数コールのコストを考える必要はないかと思われます)

\Fc2blog\Model\Model::load(\'BlogPlugins\')->findByDeviceTypeAndCategory($this->getDeviceType(), \Fc2blog\Config::get(\'BLOG_PLUGIN.CATEGORY.FIRST\'), $blog_id);

こういったコード片を

\Fc2blog\Template\Helper::getSpPluginFirst($this);

などとするということです。（究極、タグ一つにつき、１メソッドにしたほうがよいのですが、ifやforがあるので、全部は難しそうですね）

メリットとして、このPHPを生成する「文字列」が、できるだけPHPコードとして解釈できるようになり、コードの健全性をたもったり、このような圧縮した書き方をしなければならない場所で省略されがちな入力値チェックを余裕をもっていれる事が可能になります。リファクタも容易になるでしょう。

#51 マージ後に作業予定

• ユーザー側画面、グリッド切り替えチェック
• 管理画面、エディタのアコーディオン切り替えチェック
• （テスト実行前の初期データの更新）

httpd設定ミス時のコードや設定ファイルの漏洩対策として

• config系はapp以下へ移動
• index.phpは実体をapp以下に移動し、public以下に設置するindex.phpはコードを最低限に
• 他、phpファイルが公開dirにあれば対応

TBD

@fc2dev

CronでEntriesController::updateOpenStatusなどが実行されることを期待されているとおもいますが、このあたり想定する設定のドキュメントはございますでしょうか？

コードを読んで類推はできておりますが、Crontabにどのように設定されるのかよろしければお教えいただけますと幸いです。

【事象】

ダウンロードしたテンプレート、またはテンプレートプレビュー時、タグが置換されておらずタグがそのまま表示されています。

【タグ(例)】

<%topentry_description>
<%recent_image_w300>

【原因】

深く調査はしておりませんが、該当タグの置換処理がされていないように見受けられます。

現状、以下のようなデバイスが設計（設定）としては考慮されておりますが、

実際の設定や処理をみると、以下のようなコードで対応機種をしぼりこんでいるようです。

（（（
なお、テンプレートの更新系コードを調査している時にきづきました、以下のあたりで、デバイス未指定時の挙動でガラケーとタブレットがでてこないことから追いかけて気づきました

実際のところ、テンプレート編集画面でもPC,SPしかありませんし、フィーチャーフォン（携帯・ガラケー）とタブレットは明示的に切り捨てたものということでよいでしょうか？また、これらの復活の予定はありますでしょうか？

現状、テンプレート保存などでも意味なくガラケーとタブレットのテンプレートを生成したりしており、不要ならば削除したいと考えております。

（現代においては、タブレットはPC用のテンプレートでよいでしょうし、ガラケーはもはやという気がいたします）

現在毎回fc2_default_template.php修正後に管理画面からコピペして更新していますが、自動テスト時に失敗することがあります。
解決のために、\Fc2blog\Web\Controller\Admin\BlogTemplatesController::apply相当の処理をCliで受け付けるコマンドを作れればと考えています。

当座は開発・テスト用ですが、将来的にはテンプレートバージョンアップで同様の機能が必要になるかと考えています。

概要

おおまかにいえば、以下のような事がしたいと考えています。（もうすこしオプションは増減すると考えています）

$ update-template -blogid my_blog_id -templateid 1 -target pc -type template -input ./fc2_default_template.php
$ update-template -blogid my_blog_id -templateid 1 -target sp -type css -input ./fc2_default_css_sp.php

想定利用シーン

• テスト実行前のテンプレートの更新（ファイルからの最新化）の自動化

(作業時間 3h

作業

作業ブランチ： https://github.com/uzulla/fc2blog/tree/convert-to-twig

• Controller改修(作業をしながら)
• Controllerリファクタリング（作業しながら）
• layout.twig
  • PC
  • SP
• View/admin以下のテンプレートをコンバート(約100個)
  • admin/comments/*
  • admin/blogs/*
  • admin/entries/*
  • admin/tags/*
  • admin/common/*
  • admin/layouts/* (ajax,json,download.phpは不要なので移設せず）
  • admin/users/*
  • admin/files/*
  • admin/blog_plugins/*
  • admin/categories/*
  • admin/blog_templates/*
  • admin/blog_settings/*

テストを増やしてほしいというご依頼から、様々な検討をしている一環です。

以下は「管理画面」についてのことであって、動的なコードを生成する必要があり、自由度をさげたくないユーザー側の画面については検討するものではありません。

課題

現在、Web\Controller\Controller::layout、Web\Controller\Controller::displayのHTML生成は、コントローラーに設定された$this->dataをextractして現在のスコープのローカル変数化した後、テンプレートファイルをincludeして評価する形で実現されています。

これはPHPファイルをそのままテンプレートとして活用する典型的なテクニックですので、これはこれでとおもわれますが、現在テンプレート内で直接Configなどをコールしている箇所があり、複数のデータソースからテンプレートがレンダリングされているのはテストの視点からは少々問題です。

改善

Config::getなど、必要なものがあるなら必要なものを$this->dataに事前登録すべきと思われます。それによって、テストはHTMLをパースせずに生のデータ($this->data)で簡単に、正しくロジックを検証することが出来ます。

最悪でも、Configのインスタンスを$this(controller)->dataに入れておくべきで、直接Config::get(〜)といった記述はテンプレートから外すべきかなと思います。

（このために訪問者向けのレンダラメソッドと管理画面のレンダラメソッドは別になります）

さらなる改善

extract()してPHPをレンダリングする場合、テンプレートファイルから見て、呼び出されてみるまでは変数が存在するか自明ではありません。これは品質を上げるためのコードの静的解析においてはかなり厳しい条件となります。(ダミーのアノテーションを大量に書くなどのテクニックもありますが)

特に動的なPHPテンプレートが生成できる必要がない管理画面においては、PHPを直接テンプレートとせず、Twigなど安全でシンプルなテンプレートエンジンへの置き換えは検討できませんでしょうか？

テスト性も向上しますし、PHPをテンプレートエンジンとして使うのはどうやっても危険です。
（h()などを適切にいれればよいですが、ヒューマンエラーは最初からとり除くべきです）

かさねてですが、これはテンプレートを自由にいじったりしない、静的である管理画面の話であって、ユーザー側画面の話ではありません（コメント投稿画面は、そうしてもよいとも思いますが）

TBD

• テンプレート作成に失敗しても、ロールバックされない。

現状

• 基本的にトランザクションを使っていないため
• つまりトランザクションを実装すれば良い

前段でMSDBクラスと、PDO/mysqliのWrapperにおいてトランザクション周りの機能追加が必要

• MSDB(マスター/スレーブ DB？)はレプリカの存在を前提としている？（デフォルトオフ）
• PDOとmysqliのギャップを埋めるWrapperがある
• 例外は内部でキャッチされ、詳細はログにおくられ、レスポンスとしてはfalseだけが返る
  • そのまま例外があがってくれればハンドリングしやすいが…？
  • ただ、すべて\Exceptionなので、\PDOExceptionなどにしたい気はする（ただ、デュアルサポートがきびしくなる）
• レプリカではトランザクションを明示的に拒否する（書き込みは発生しないはずなので）機能などが必要そう
• SQL文を流すことでもBeginやCommitは可能だが、通常DBドライバのメソッドを使うので、その機能が必要そう

検討

• メイン/レプリカ構成をサポートするWrapper（というより、DB接続の管理）は残すことは特に問題ない
• mysqliとPDOデュアルサポートをしているが、本改善や今後のサポート工数の削減を考えると、どちらかに寄せても良いのではないか？
  • 昨今のPHP実行環境においては、mysqliやPDOどちらかない実行環境、というのは特殊かと思われる。
• 現在のコードではmysqli優先となっている。それでもかまわないかと思います
  • いまはデュアルサポートしているので、mysqliならではの機能をつかっているわけではない

現状のDBまわりの別の問題点

• 万能関数resultなどに\Fc2blog\Model\DBInterface::RESULT_ONE, \Fc2blog\Model\DBInterface::RESULT_LISTさまざまな返り値の形がオプション指定ができるため、帰ってくる値の型が深い所の評価まで確定しないので、静的解析がききづらく、エラー処理を書いていて「なにがエラーでなにが正しいのか」がわかりづらい。@return が mixed|int|false|array などになってしまう
  • エラーハンドリングを例外でなく返り値で行うのはスタイルなのでそれはそれでと思うが、完全に最適化してもint|falseなどになるので、将来的なPHP(静的解析)には厳しいのではないか。
• 愚直に取得される型別に、別のメソッドにした方が良いのではないか。
  • そういうWrapper関数を生やすことはできるかと思う

【事象】

1. テンプレート新規作成または編集から<%comment_reply_body>を含まないテンプレートを追加/更新
   ※作成/更新したテンプレートを適用せず、初期テンプレートを適用のままにしておく
2. 管理画面からコメントに対して返信する
3. ブログ詳細画面のコメント一覧に下記のように表示される（例）
   タイトル：Re: タイトル
   コメント：管理者からの返信です。
   管理者からの返信：管理者からの返信です。

【原因】

テンプレート作成/更新時にコメントの表示タイプが更新されており、HTMLに<%comment_reply_body>を含まない場合は1が登録されます。
コメントの表示タイプが1の場合、選択されているテンプレートに関係なく本不具合の内容でHTMLが出力されます。

【DB】

blog_settingsテーブル->template_pc_reply_type

【コメント表示タイプの更新箇所】

app/model/comments_model.php
->updateReplyType

この削除をクリックすると、

編集画面へ遷移してしまう

現状基本的にBlogsModelなどから取得する「インスタンス」はfetchした行なので、記述されているデータが自明でないことが多く、見通しがよく有りません。

例として、以下は「ランダムに１件ブログを取得する」メソッドですが、この返り値はblogsテーブルの１行配列ですが、コードからどのようなカラムがあるのかは類推できません。

一般的に、カラムに対応するプロパティをクラスとして書き換えることで見通しがよくなります。

ただ、既存コードが配列に強く依存しているため、全体の書き換えはかなりの工数がかかります。

提案

ArrayInterface、およびIteratableを実装した配列としてアクセス可能なクラスを実装し、各種返り値を可能なタイミングでそれに差し替え、順次型をつけて見通しがよくするステップをふむことで、コードを長期間フリーズせずに順次移行ができるかなと考えております。

移行中はArrayとRowクラス混在してしまうのは難点ですが、とりあえずその場合でもArrayとしてあつかって書いておいてもらえれば動くには動くのと、現状のテーブル数は13なので作るだけ作るのは可能と考えております。

とりあえずビジネスロジックはかえず、要所にアノテーションを追記する形で静的解析をきかせてバグを減らし、開発の省力化ができます。

なお、典型的にはModelという命名が多いですが、これはすでにつかわれているので、Model\Row\BlogRowなどの命名になる想定です。

改修後は

現状、暗黙になっている以下のようなコードが

  public function switchTemplate(BlogRow $blog_template, string $blog_id)
  {
    $device_type = $blog_template->device_type; // device_typeはBlogRow内でintと型定義

などと型付けができるようになることを想定しています。

一部はそのまま配列にする予定です

たとえば、以下のようなものは当座そのまま配列にしておく予定です。（データ量が大きくなるかもしれないですし、しぼりこんでいるものを広げると、なにか悪影響があるかもしれませんので）

自動的にアップデートする仕組みを実装する

工程

• お知らせ機能
• メンテバージョンがあげられる機能（バグ修正のみ（つまり、app以下にファイルを置き換えるだけ）、テンプレやDB等に変更が不要）
• マイナーバージョンがあげられる機能（非破壊な機能追加のみ、DBはカラム追加のみ、テンプレ再生成は必要な範囲か？）
• メジャーバージョンがあげられる機能（コンフィグファイルの設計や、DBでカラム削除まであるようなスキーマ変更あり）

検討課題

• 自動更新ができる「範囲」について
  • メンテバージョンアップ
  • マイナーバージョンアップ
  • メジャーバージョンアップ
  • どこまで「自動」でできることを目指すか？
• ロールバックは必須か？
• 互換性確認
  • プラグイン的なものが始まると大変
• コード更新方法
  • releaseのtar ballを展開する
    • アップデート中は「壊れた」状態になりがち
    • 「メンテナンス中」表示機能
  • サーバー上でdeployer(等)のデプロイツールを自己実行
    • レンサバと相性が悪い
    • ストレージを多く消費する
  • ブログアプリとインスタンスを分離し、composer update する
• DBマイグレーション
  • 素朴なマイグレーションSQLを同梱
  • phinxなどのマイグレーションツールの導入
  • 実行時間制限で失敗すると壊れる
  • DB設計を変更し、「カラムの追加」がほとんど発生しない設計に変更する
• （現在の実装に存在する）CSSなど、依存する生成ファイルの類をどうするか？
  • 毎回再生成？
• アップデートのトリガーは？
  • 管理画面で実行
  • 新バージョンが存在する事をユーザーに通知する仕組み
  • ユーザーアクセス（で、ランダムに確認させる？）
  • cron
• 間隔がリープしたバージョンアップのテストをどうするか？
  • 「1.0.0から1.29.9 までバージョンアップできる」ことをどうテストするか？
  • あるいは順番にしかあたらないようにするか？

#35 (comment) より

JSのCookieライブラリ更新

• 現行 jQuery Cookie Plugin v1.4.0（https://github.com/carhartl/jquery-cookie ）は古く、更新されていない
• （たとえば）後継として指定されている js-cookie v2.2.1（https://github.com/js-cookie/js-cookie ） などで置換する

しないこと

• 大幅なJavascript周りの改修（ビルド・バンドル化など）
• 現行から大幅に書き変わる対応（できるかぎり置換的な対応をする）

【事象】

1. 管理画面のプラグイン作成/編集へ遷移
2. 「タイトル文の文字色、文字位置」の色から「無し」以外を選択
3. 「コンテンツの文字色、文字位置」の色から「無し」以外を選択
4. ブログTOPを確認
5. タイトル文、コンテンツ共に選択した色が適用されていません。
   ※デフォルトテンプレートを使用
   ※文字位置の設定は反映されていました。

【原因】

未調査

#53 より

admin/installer.phpを利用後「削除してください」というのはいかにも面倒なので、インストールがおわったら自動的にうごかなくする機能があれば便利だと思われる。
（ID/PASSを忘れた時のレスキュー用途かとおもっていたが、特にそういうことはないようなので）

installer.phpを削除してしまう（必要なら、再度アップロードさせる）
「インストール済み」を示すような…
app以下になにかロックファイルを作る（削除すれば、また動く）

ご回答いただいたのでロックファイルの実装をする。

概要

fc2blogのオープンソースのPHP7化対応を行った際の調査、対応、テストなどをまとめます。
現在PHPはバージョン5.2.17 or higher、MySQLはバージョン5.1 or higherで担保されていますが、PHP7.4.x(2020年7月時点の最新バージョン)以上で動くよう対応しました。
また、その対応による動作検証も合わせて行っております。

対象コード

https://github.com/fc2blog/blog

調査

スクリプトによる該当箇所調査

php5.2.x->7.x.xの互換性調査として 『PHP 7 Compatibility Checker 1.2.1』を用いてコード全体のチェックを行いました。

<スクリプト実行結果>

XXX-MacBook-Pro:task {name}$ php7cc.phar blog/

File: /Users/{name}/Desktop/task/blog/public/js/elrte/src/post.php
> Line 2: [Error] Removed function "set_magic_quotes_runtime" called
    set_magic_quotes_runtime(0);

Checked 75 files in 0.971 second

上記の通り、set_magic_quotes_runtimeメソッドのみ引っかかりました。

手動による該当箇所調査

スクリプト実行では対応不可である最新バージョンの確認や、また万が一の対応漏れがないようにするため、手動でもphp5.2.x->php7.4.xまでの変更点全てを調査しました。
fc2blogコード手動調査.pdf
※弊社のコンフルにまとめてあるものをPDFとしてexportしたため、多少の表示崩れがありますことご留意ください。

スクリプト実行と手動調査による調査結果

下記3点の修正が必要ということが判明

1. 波括弧を使った、配列や文字列のオフセットへのアクセス

   
 - https://www.php.net/manual/ja/migration74.deprecated.php
2. countable ではない型をカウントしたときの警告
   
 - https://www.php.net/manual/ja/migration72.incompatible.php
3. set_magic_quotes_runtimeメソッドの廃止
   
 - https://www.php.net/manual/ja/function.set-magic-quotes-runtime.php

実装

実装内容(diff)

既にmasterブランチにマージ済みであるため、forkした弊社のリポジトリにおけるプルリクエストに加え、diffをテキスト形式でも添付させていただきます。
samoslink001#1
gitdiff.txt

テスト

結合テスト

対象ページのテストは全て通過しました。
何箇所か結果が x となっている箇所がありますが、こちらがお伝え済みの既存バグが原因のものです。
fc2blog_php7_結合テスト.xlsx

性能テスト

• 1秒間に121.4回読み込みと実行が可能なので実用性に問題無し
• 1度の読み込みと実行が0.008233秒で処理が完了するため問題無し
  fc2blog性能確認テスト.pdf

その他資料

対応スケジュール

fc2blog_PHP7化対応スケジュール表.xlsx

configディレクトリには用途が異なるファイルが多くあり、分類や分離、統合をしたほうが良いのではないかと考えています。

## Config系ファイル
 → 引き続きConfigでよいかと思う。ただ、app.php、dir.php、 env.phpは集約してよいのではないか？別にしている意義が読み取りづらい
app.php
dir.php
env.php

## なかばロジックとなっているConfig
 → 環境の分岐のためにある。当座はconfigでよいかと思うが、コード化すべきと思う
admin.php
cron.php
test.php
user.php

## コードとなっていないのが不思議なConfig
  → 参照箇所が少ないので見通しを良くするために、コードにしたほうが良いかと思う
fc2_request.php

## ロジックが含まれているので特殊だが、Configとも言えるもの
 → コード化、あるいはコードと設定を分離すべきと思う
routing.php
routing_test.php
routing_user.php

## プラグインのスニペット（的な）集
 → これはConfigよりもテンプレート寄りのものかとと考えている、templateやpluginなどのディレクトリへ移設するのはどうか
（将来的にどのように増えていくのか？で変わるのかなと思っている）
fc2_default_plugin.php
fc2_template.php

## デフォルトで使われるテンプレート
 → templates/defaultなどのディレクトリをつくり、移動すべきかと思う
（ただ、実体はDBにもあるので、blog_template tableを主とするなら、`sample/template`でもよいかと思う）
fc2_default_css.php
fc2_default_css_sp.php
fc2_default_template.php
fc2_default_template_sp.php

## 初期化用のDDL、configではない。
 → 今後DBのマイグレーションを考えると `/resource/db/0_init.sql`などに移動など
blog.sql

なにかしら意味があれば変更はむずかしいとおもいますが、ご検討いただければ幸いです。