LoginSignup
6
1
しくじりエンジニア!私みたいになるな!
Qiita Engineer Festa20242024年7月17日まで開催中!

【自戒】よく知らないライブラリを放置するな

Last updated at Posted at 2024-06-14

導入

こんにちは。元バックエンドエンジニアのYamaです。
過去に辛い出来事があったので、記事にしました。
この記事を持って供養できたらと思います。

201X年XX月XX日

コロナウィルスという言葉がまだ存在していなかった頃、
この頃、私はSpringBootで開発されたアプリケーションの保守を行なっていました。
アプリケーション自体は何の変哲も無い、CRMシステムでした。
少し変わった点といえば、もともとの開発業者が多重下請け構造という闇を生み出しており、その業者の担当者は仕様を十分に理解せず、ドキュメントもほとんど作られていないという状態でした。
コミットの作成者を見ると様々なドメインのメールアドレスが並んでいたと記憶しています。
現在は減ってきているとは思いますが、当時は別に普通というか、よくある状態でした。
一部、中国語のコメントなどが残っていたり、日本語が怪しかったりしましたが、当時はそこまで気にしていませんでした。

この日も掛け持ちしている開発プロジェクトの開発を終え、会社を後にしました。
電車に乗り2駅ほど過ぎたくらいの時でした。

19:30

いつも使っていない社用携帯がなりました。
「いつも使っていない」という接頭語が付くくらい出番のない社用携帯でした。
この時点で、私の携帯電話が鳴っていることに気づけないくらい稀有な出来事でした。
ちょうど次の駅に到着したので、電車を降りながら、着信元を確認します。
上述のCRMシステムを共に担当していた後輩のHさんでした。
胸がざわつきました。
私と同じタイミングで降車した人や乗車待ちをしている人がホームに結構いたため、私は電話に出ながら、
「数秒待ってね」
と話したことを記憶しています。

19:35

ホームでは、電車から降りて階段に向かっていく人波がありました。
私はそれに対し逆に進行し、人の少ないところで、改めてHさんに声をかけました。
「今電車を降りたところ。どうかした?」
Hさんは返します。
「FuMiSanさん、、やばいです」

20:XX

状況を確認し、すぐに会社に戻りました。
すでに正門は閉まっていたので、裏口から会社に入りました。
開発ルームに戻ると、HさんとCRMシステムの責任者のTさんがいて、Tさんは関係各所に連絡をしている様子でした。

どうやら以下の問題が発生しているようでした。
(テーブル名などは少し変更しています。)

  • Userテーブルのいくつかのカラムの値が、ある特定の値に書き換えられている
  • その影響でユーザがログインできない状態になっている

この時点では原因は不明でしたが、
ユーザに対するインパクトも影響範囲も大きいため、Tさんと会話し、サービスを緊急停止することにしました。

20:3X ~

緊急停止後、DBの状態を改めて確認します。
書き換えられたレコード(全レコード)の更新日時が私が会社を離れる10分ほど前になっており、更新者がある一人のユーザ名になっていました。
アプリケーションログからそのユーザの行動を追っていきました。
(当時はオブザーバビリティといった言葉もあまり聞くことがなく、その考え方がこの問題が生じる前に我々の頭の中にあればもう少し調査時間を減らせたのではないか、と思ったりします。)
調査と並行して、DBは復元できるようにスナップショットを復元しておきました。

21:XX

調査の末、
「更新者であるユーザの自身の情報変更を行なった際、特定の条件が揃っていると、全レコードを上書きしてしまう」という潜在バグが存在していたことが判明しました。

長くなってしまったので、整理

  • 前担当者に聞いても仕様もわからない
  • ドキュメントもない
  • ある条件下でユーザ情報更新がかかると、他の全ユーザのレコードが更新されてしまうという潜在バグがあり、それがこの時発生した。

原因

結論から言うと、MyBatisPlusというライブラリの扱いが良くなかった、ということになりました。

MyBatisPlusとは

MyBatisPlusとは、JavaのデータアクセスフレームワークであるMyBatisを拡張したライブラリです。MyBatisよりも簡単なCRUD(Create, Read, Update, Delete)操作を可能にしてくれるライブラリです。

使用例

※ここはもう使い方を忘れているので、ChatGPTに生成してもらったサンプルです。

エンティティクラスの定義

テーブルに合わせたエンティティクラスを定義しておきます。

User.java
@Data
@TableName("user")
public class User {
    private Long id;
    private String name;
    private Integer age;
    private String email;
}

マッパーインターフェースの定義

UserMapper.java
public interface UserMapper extends BaseMapper<User> {
}

使用

UserService.java
@Service
public class UserService {
    @Autowired
    private UserMapper userMapper;

    public List<User> getAllUsers() {
        return userMapper.selectList(null);
    }

    // IDリストに合致するユーザーのageを更新
    public void updateUsersAgeByIds(List<Long> ids, Integer newAge) {
        UpdateWrapper<User> updateWrapper = new UpdateWrapper<>();
        // ***** 問題の処理 *****
        updateWrapper.in("id", ids).set("age", newAge);

        // 更新処理の実行
        userMapper.update(null, updateWrapper);
    }
}

以上のようにJavaのみでクエリを記述することができ、Dao.xmlファイルを作成し、その中にSQLを書かなくても良いというメリットがMyBatisPlusにはありました。

ただ、私はMyBatisPlusをよく知らなかった

はい。この項目の通り。私はMyBatisPlusが便利なライブラリという認識しかありませんでした。
ましてや問題点があることなど考えることがありませんでした。

しかし、上述のコードにも書いた通り「問題の処理」が存在しました。

問題点について

ほとんど答えのような恐ろしい質問ですが、SQLのin句の中身が空であった場合、クエリの実行対象はどうなるでしょうか?:angel:
MyBatisでは試していませんが、SQLだとこうなります。

select * from user where id in ();

当然、Syntax Errorになるはずです。
では、MyBatisPlusだと、どうなるでしょう。
 
...
 
はい、お察しの通りでございます。
条件が吹っ飛びます。つまり、

select * from user;

と、あるべきだった条件がなくなるのです。
あえて、Selectで書きましたが、これがUpdateだとどうでしょうか?
Setされる対象を絞り込むための条件がなくなるんです。
そうです。全レコード更新が起こるわけです。

対応

すぐに当該処理を修正し、全レコードが更新されることがないように修正し、リリースしました。
幸い時間が遅い時間だったため、利用者数も少なく、実害は大きくありませんでした。
利用者の多くが品質の悪いシステムであることを以前から認識していたこともあり、無事終息させることがありました。

教訓

さて、この問題で私が当時得た教訓はタイトルにある「よく知らないライブラリを放置するな」です。
当時、私もHさんも「動いているから、ヨシ!」といったノリで、あまり元々使われているライブラリを意識していませんでした。
今ではもちろん新規プロダクト開発でライブラリ導入時や保守において、

  • メンテナンス状況
  • 脆弱性の存在
  • コミュニティの活発さ
    を意識するようにしています。
    余力があれば、ライブラリの実装自体を確認するように心がけるようになりました。

最後に

皆さんもよく知らないライブラリを放置せず、疑いの目を持って接してみてもらえたらと思います。
第二、第三の私が誕生しないことを願って。。

6
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
6
1