php / php/php-src

file_put_contents() is racy

オープン
#20,108 コメント 8 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
C
スター
40.4k
フォーク
8.1k
平均マージ
2日 13時間
マージ済み PR(30日)
96

説明

https://github.com/php/php-src/blob/18d99ee4b724f9ecd18122abacfac5767bfbfb48/ext/standard/file.c#L492

Imagine following sequence (two processes A and B writing some serialized data to file called "cache.dat" using file_put_contents():

  • Process A: open the cache file for writing
  • Process B: open the same cache file for writing
  • Process A: truncate the file to zero length
  • Process B: truncate the file to zero length
  • Process A: write the cached data (e.g. 1002 byte long serialized data)
  • Process B: write the cached data (e.g. 1000 byte long serialized data)
  • Process A: close the file
  • Procees B: close the file

This results in a file with 1000 bytes from Process B and 2 extra bytes from process A and you end up with unserialize(): Extra data starting at offset 1000 of 1002 bytes if you later try to unserialize the file.

Since it seems that many developers incorrectly believe that a function called file_put_contents() would actually write a file with the given contents, I'm suggesting that PHP internal implementation should instead be (unless FILE_APPEND or LOCK_EX in flags)

  1. Create a new temporary file in the same directory with the target filename.
  2. Write the given data (argument to file_put_contents()) into said file.
  3. Close the file
  4. Rename the temporary file to final target filename.

Since only the rename() is guaranteed to be atomic in POSIX compatible systems (and even that requires rename within the same directory), this is the only way to make sure you don't end up with mixture of two files when multiple processes are calling file_put_contents() at nearly the same time. And in case NFS is used, you cannot assume that LOCK_EX actually works so you must use rename() semantics.

And maybe allow current behavior with some new flag (FILE_ALLOW_RACY?) with documentation "Reduce syscalls to improve performance but caller must guarantee that two processes are not trying to write into the same file concurrently."

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

ext/standard/file.c の492行目付近から始め、通常の書き込み、FILE_APPEND、LOCK_EX に対する file_put_contents() の処理を追跡してください。提案されている同一ディレクトリ内の一時ファイル作成と rename のシーケンスを確認し、同時書き込みに必要な動作とカバレッジを判断してください。これには FILE_ALLOW_RACY の代替案とドキュメントも含まれます。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
c, php
領域
operating-systems
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。