php / php/php-src

file_put_contents() is racy

Abierto
#20,108 8 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
C
Estrellas
40.4k
Forks
8.1k
Merge medio
2 d 13 h
PR fusionados (30 d)
96

Descripción

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."

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza en ext/standard/file.c alrededor de la línea 492 y sigue el manejo de file_put_contents() para escrituras normales, FILE_APPEND y LOCK_EX. Revisa la secuencia propuesta de archivo temporal en el mismo directorio y renombrado; después, determina el comportamiento requerido y la cobertura para escrituras concurrentes, incluida cualquier alternativa FILE_ALLOW_RACY y la documentación.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
c, php
Área
operating-systems
Tipo de issue
Nueva funcionalidad
Dificultad
5/5
Tiempo estimado
Más de una semana
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.