Home > Net >  fstream is writing on file against an if else statement
fstream is writing on file against an if else statement

Time:03-04

I have a file named "password.txt" which has 2 rows of usernames and passwords

2 string vectors named, user & password

This function uses a new username and password and checks if the username already exists in the user vector using a for loop

If it does exist, it breaks out of the loop If it does not exist, it will add the username and password to the vectors and write them both on the file

void PasswordFile::addpw(string newuser, string newpassword) {
    fstream file;
    file.open("password.txt", ios::app);
    
    //if the vectors is empty it automatically adds a new user
    //is working
    if (user.size() < 1){
        user.push_back(newuser);
        password.push_back(newpassword);
        file << newuser << " " << newpassword << "\r";
    }

    //the loop (is not working)

    for (int i=0; i < user.size(); i  ){
        string name = user[i];
        if (name == newuser) {
            break;
        } //it shouldn't continue past this point if the name is in the vector but it does

        else {
            cout << newuser;
            cout << " is being written onto the file\n";
            user.push_back(newuser);
            password.push_back(newpassword);
            file << newuser << " " << newpassword << "\r";
        }
    }
    
    file.close();
}

the problem is that it is writing the second name twice and the third name three times

I know also that it is pushing the newuser and newpassword multiple times onto the vector, but I can't figure out where or why its looping more than it should or why its ignoring the break and writing onto the file anyway

"password.txt" 1 run

dbotting 123qwe
egomez qwerty
tongyu liberty
tongyu liberty

"password.txt" second run

dbotting 123qwe
egomez qwerty
tongyu liberty
tongyu liberty
egomez qwerty
tongyu liberty
tongyu liberty

it keeps adding the last 2 every time its run

CodePudding user response:

WHILE you are looping through the user vector, any time you encounter an entry that does not match the newuser being searched for, you are writing the newuser/newpassword to the file and pushing them into the vectors 1 (which then affects subsequent iterations of your loop), and then you move on to the next entry.

So, for example, if you start with a file like:

dbotting 123qwe
egomez qwerty

tongyu does not match dbotting, so you add tongyu. And then tongyu does not match egomez, so you add tongyu again. So you end up with:

dbotting 123qwe
egomez qwerty
tongyu liberty
tongyu liberty

And then on the next run, tongyu doesn't match dbotting or egomez, so you add tongyu twice again. But then it does match tongyu, so you don't add it again, and break the loop. So you end up with:

dbotting 123qwe
egomez qwerty
tongyu liberty
tongyu liberty
tongyu liberty
tongyu liberty

And so on.

Online Demo

To fix this, the code inside of your else block needs to be moved out of the for loop entirely. You need to search the entire vector to determine if the user already exists or not, and then write that user to the file only if they don't exist at all, eg:

void PasswordFile::addpw(const string &newuser, const string &newpassword) {
    for (size_t i = 0; i < user.size();   i){
        if (user[i] == newuser) {
            cout << newuser << " already exists" << endl;
            return;
        }
    }

    /* alternatively:
    if (find(user.begin(), user.end(), newuser) != user.end()) {
        cout << newuser << " already exists" << endl;
        return;
    }
    */

    ofstream file("password.txt", ios::app);
    if (!file.is_open()) {
        cout << "cannot open password file" << endl;
        return;
    }

    cout << newuser << " is being written onto the file\n";
    if (!(file << newuser << " " << newpassword << "\r")) {
        cout << "cannot write to password file" << endl;
        return;
    }

    user.push_back(newuser);
    password.push_back(newpassword);
}

1: On a side note, you should not maintain two separate vectors that are closely related. Just use a single vector instead, eg:

struct UserInfo
{
    string username;
    string password;
};

std::vector<UserInfo> users;

void PasswordFile::addpw(const string &user, const string &password) {
    for (size_t i = 0; i < users.size();   i){
        if (users[i].username == user) {
            cout << user << " already exists" << endl;
            return;
        }
    }

    /* alternatively:
    if (find_if(users.begin(), users.end(), [&](const UserInfo &u){ return u.username == user; }) != users.end()) {
        cout << user << " already exists" << endl;
        return;
    }
    */

    ofstream file("password.txt", ios::app);
    if (!file.is_open()) {
        cout << "cannot open password file" << endl;
        return;
    }

    cout << user << " is being written onto the file\n";
    if (!(file << user << " " << password << "\r")) {
        cout << "cannot write to password file" << endl;
        return;
    }

    UserInfo newuser;
    newuser.username = user; 
    newuser.password = password;
    users.push_back(newuser);
}

CodePudding user response:

Remy has done a good job of pointing out the problems you're seeing.

Rather than fixing your loop, I would fix the data structures you're using.

Instead of a pair of vectors, one for user names and one for passwords, I'd start by creating an std::map to hold the usernames and passwords:

std::map<std::string, std::string> credentials;

A map already provides most of what you want, as far as guaranteeing that an entry is unique (each username occurs only once), and lets you check whether an insertion succeeded, so an equivalent of your addpw comes down to something like this:

std::map<std::string, std::string> credentials;

void addpw(std::string newuser, std::string newpass) {
    if (credentials.insert({newuser, newpass}).second) {
        std::cout << "Adding new user: " << newuser << "\n";
        file << newuser << " " << newpass << "\n";
    } else {
        std::cerr << "User already exists--not adding\n";
    }
}

Of course, if you care about security at all, you really don't want to store user names and passwords in a plain text file. In fact, you usually don't want to store actual passwords at all--as a bare minimum, you want to salt the password, the hash it with a cryptographic hash algorithm (e.g., SHA-256) then store the salt and the hash, not the password itself. Then to check a user's credentials when they try to log in, you salt what they provide, hash it, and compare the resulting hash to the one you have on file and see if they match.

Now don't get me wrong: that's not the ideal to provide truly maximum security--but at least it's enough to make hackers work a little bit to steal your users' credentials.

  •  Tags:  
  • c
  • Related