Saving form data to a text file on server

Question

I want to save simple string data as a simple text file (.txt) on the server (together with the IP), but I cannot find any clear guideline on how to do this correctly/efficiently/safely. (Maybe I'm searching with the wrong keywords..)

Here is a code I came up with, it works well for storing but I'm not sure whether it's ideal otherwise.

<?php

ini_set("display_errors", 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

function test_input($data){
   $data = trim($data);
   $data = stripslashes($data);
   $data = htmlspecialchars($data);
   return $data;
}

// get IP
if(!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
    $user_ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
} else {
    $user_ip = $_SERVER['REMOTE_ADDR'];
}

// sanitize IP -- not sure if necessary, but why not
$user_ip = test_input($user_ip);

$postdata = file_get_contents("php://input");

$request = json_decode($postdata);

// assign and sanitize input values
$file_name = './' . test_input($request->fname_post);
$user_data = test_input($request->results_post);
$user_data = str_replace("UNKNOWN_IP", $user_ip, $user_data);

file_put_contents($user_ip . $file_name, $user_data);

// check if successfully saved, plus sanity check for file name (extension)
if ($outcome > 500 AND substr($file_name, -4) === ".txt") {
    echo "Success";
} else {
    echo "Failed"; // (here I'll actually include more info, but that's not important for the main question)
}

?>

Is this a sensible and correct way to do it?

EDIT:

Some more details about the use case.

• I need to be able to rewrite the file (when the same user resends the data), hence I cannot use random file name. Randomization does happen though on the client side, so others can hardly guess the file names. Including IP in the file name is another way to make this less problematic (plus its a convenience for management reasons). Also, losing this data would not be catastrophic; when the user finalizes the whole form, the full data is saved separately in another file via file_put_contents($file_name, $user_data, FILE_APPEND | LOCK_EX);, so it cannot be overwritten. If there are other issues with fixed files names apart from potential overwriting, please explain.
• I need to save the file as .txt with the content in a specific structure (as sent from the user).
• Even if the data is not as expected, it should be saved, hence the $outcome > 500 should not prevent saving, it's used only for feedback, also because, as far as I understand, $outcome could still exist e.g. if the server space ran out and only an empty file was "stored".

Answer 1

All I can do is repeat what I already told you on StackOverflow: this code is crippling the data and stores a fake IP address.

The main problem with your code is that you don't understand what does it do. You are just parroting some code you found, but never question whether it makes any sense or serves any purpose you need.

Take, for example, that test_input function which you apparently take for some sort of a magic wand that one can tap any data with and make it magically "safe". It's time to learn that there is no magic, and every tool is only useful if used on purpose. This function does nothing useful at all, let alone your particular case.

The same goes for getting the IP address. Despite the fact that HTTP_X_FORWARDED_FOR can be easily spoofed, for some reason you are using it anyway.

Using user supplied filenames is always dangerous. Better to generate a random one.

Other minor issues:

• "display_errors" must be off on a live server, for security reasons
• 'display_startup_errors' hardly makes any sense at all
• verifying the user-entered data after writing it on a disk is a bit late.

To answer a generic question you asked, nothing complex is needed. The only protective measure you need is to store the user-provided files outside the web-root. And then just write your file right away. So it will be just a single line:

file_put_contents("$path/$file_name", $user_data);

Regarding your code, I wouldn't write the user IP in the file name but rather into the file.

So the code would be

<?php
ini_set("display_errors", 0);
error_reporting(E_ALL);

$postdata = file_get_contents("php://input");
$request = json_decode($postdata);
if (strlen($request->results_post ?? '') < 500 or substr($request->fname_post ?? '', -4) !== ".txt") {
    die("Failed");
}

$request->user_ip = $_SERVER['REMOTE_ADDR'];

$path = $_SERVER['DOCUMENT_ROOT']."/../files/";
$filename = $path.uniqid(rand(), true) . '.json';
$result = file_put_contents($filename, json_encode($request));

if ($result) {
    echo "Success";
} else {
    echo "Failed";
}

Answer 2

User input

YCS has already covered many aspects of the code in terms of taking user input. I agree that using a random string instead of the IP address is likely better for the filename, since the user could alter the value.

If the IP address is taken from $_SERVER['HTTP_X_FORWARDED_FOR'] then it could be filtered and validated using filter_var() with filter ID FILTER_VALIDATE_IP. As YCS mentions at the end of this review there is no point in using that on $_SERVER['REMOTE_ADDR'].

Operator Precedence

PHP has two variations for logical AND i.e. and and && (as well as for logical OR: or and ||). Note that and has lower precedence than many other operators including assignment - =.

There is a place near the end where AND is used:

// check if successfully saved, plus sanity check for file name (extension)
if ($outcome > 500 AND substr($file_name, -4) === ".txt") {

This line doesn't appear to be problematic, though as this SO answer illustrates it is wise to use caution and consider which operator variations to use thoughtfully. See also this article by YCS about this matter.

It appears that $outcome is not being assigned in the code posted above. Was it removed after pasting it into the question? Is it supposed to be assigned to the return value from the call to file_put_contents()

Closing PHP tag

The last line is a closing PHP tag, which isn't bad but as long as there is only PHP code in the file then it can be left out. The PHP documentation for the PHP tags states:

If a file contains only PHP code, it is preferable to omit the PHP closing tag at the end of the file. This prevents accidental whitespace or new lines being added after the PHP closing tag, which may cause unwanted effects because PHP will start output buffering when there is no intention from the programmer to send any output at that point in the script.