2

I'm making a password generator. Trying to implement pass-phrases.

The function loadFile(); won't return a value to my main function, generate();

I think I'm too close to the problem and can't logic it out. I could use some help. Thank you.

Test site: https://jf0.000webhostapp.com/passWordGenerator/ test.js source: https://jf0.000webhostapp.com/passWordGenerator/test.js

The source is getting kind of long, sorry.

//Main generate password function
function generatePassword(){
    var x = document.getElementById("passOutput");
    var p = document.getElementById("testP");
    x.value = generate();
    //Make sure they aren't using an insecure number of characters
    checkMaxChars();
    p.innerText = x.value;
}

//Generate a password. 3 passSets for customization, one for passphrase
function generate(){
    var nL = document.getElementById("noLetters");
    var nN = document.getElementById("noNumbers");
    var nS = document.getElementById("noSymbols");
    var pPK = document.getElementById("passWordPhrase");
    var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
    var numbers = "0123456789";
    var symbols = "!@#$%^&*()_+~`|}{[]:;?,.-='";
    var maxChars = document.getElementById("maxCharControl").value;
    var generatedPass = "";
    var holdPass = "";
    //Main loop
    for(var i = 0;i < maxChars;i++){
        //Pick random value for each passSet
        var passSet1 = chars.charAt(Math.random() * chars.length);
        var passSet2 = numbers.charAt(Math.random() * numbers.length);
        var passSet3 = symbols.charAt(Math.random() * symbols.length);
        //if a checkbox is selected, clear out that value
        if(nL.checked == true){passSet1 = "";}
        if(nN.checked == true){passSet2 = "";}
        if(nS.checked == true){passSet3 = "";}
        //Randomly select a set to be added to holdPass, or not if it is empty.
        var r = Math.floor((Math.random() * 4) + 1);
        if(r == 1){if (passSet1 == ""){i--}else{holdPass += passSet1;}}
        if(r == 2){if (passSet2 == ""){i--}else{holdPass += passSet2;}}
        if(r == 3){if (passSet3 == ""){i--}else{holdPass += passSet3;}}
        if(r == 4){
        if(pPK.checked == true){
            //get the value from loadFile(); and apply it to passSet4, add that to holdPass.
        }}
    }
    generatedPass = holdPass;
    console.log("Max Characters:" + maxChars);
    console.log("passSet1:" + passSet1);
    console.log("passSet2:" + passSet2);
    console.log("passSet3:" + passSet3);
    console.log("Iteration:" + i);
    console.log("Random Position:" + r);
    console.log("Password:" + holdPass + "::::" + holdPass.length);
    //return password
    return generatedPass;
}

//Make sure they didn't select all the checkboxes
function checkBoxes(){
    var nL = document.getElementById("noLetters");
    var nN = document.getElementById("noNumbers");
    var nS = document.getElementById("noSymbols");
    var pP = document.getElementById("passWordPhrase");
    var nA = document.getElementById("noticeArea");
    var nT = document.getElementById("noticeAreaText");
    if(nL.checked == true && nN.checked == true && nS.checked == true){
        nL.checked = false;
        nN.checked = false;
        nS.checked = false;
        nA.style.display = "block";
        nT.innerHTML = "You cannot select all checkboxes at once.";
        window.setTimeout(hideNotice,5000);
    }
    else{
        nA.style.display = "none";
        nT.innerHTML = "";
    }
}
//make sure the max characters is greater than 8
function checkMaxChars(){
    var maxChars = document.getElementById("maxCharControl").value;
    var nA = document.getElementById("noticeArea");
    var nT = document.getElementById("noticeAreaText");
    var x = document.getElementById("passOutput");
    console.log(maxChars);
    if (maxChars < 8){
    x.value = "";
    nA.style.display = "block";
    nT.innerHTML = "You cannot generate a password less than 8 characters in length for security reasons.";
    window.setTimeout(hideNotice,7000);
    }
}
//hides the notice area div once the message is completed
function hideNotice(){
    var nA = document.getElementById("noticeArea");
    var nT = document.getElementById("noticeAreaText");
    nA.style.display = "none";
    nT.innerHTML = "";
}
//Load file
function loadFile() {
    var xhttp = new XMLHttpRequest();
    var x;
    xhttp.onreadystatechange = function() {
    if (this.readyState == 4 && this.status == 200) {
        x = this.responseText;
        parseResponse(x);
        }
    }
    xhttp.open("GET", "wordList.csv", true);
    xhttp.send();
    return x;
}
//Format response
function parseResponse(x){
    console.log("MADE IT HERE!");
    var dS,sV1,rPos;
    dS = x.split(",");
    for(var i =0;i < dS.length;i++){
        sV1 = dS[i];
    }
    x = sV1;
}
pppery
  • 3,731
  • 22
  • 33
  • 46
  • You are attempting to return `x` before the AJAX request has finished. You need to move that line into your AJAX success handler. – Scott Marcus Aug 01 '17 at 20:33
  • What Scott said, you should be waiting for a response before running your callback. I would recommend using a fetching library such as axios for this. – Brady Edgar Aug 01 '17 at 20:37
  • @BradyEdgar Why would you recommend a 3rd party library for that? Just deal with the result in the success callback. – Scott Marcus Aug 01 '17 at 20:40
  • @ScottMarcus I am speaking more in general, when handling ajax calls and async calls it is much more stable and easier to read when using something like axios or fetch` over using `XMLHttpRequest `. – Brady Edgar Aug 03 '17 at 15:55
  • @BradyEdgar I don't see how that would be on any benefit. Adding a library that simply wraps native behavior isn't going to get the OP to understand how AJAX operations work. – Scott Marcus Aug 03 '17 at 16:26

2 Answers2

1

The results of an AJAX request will never be available until after the function that initiated it completes. You are attempting to return x before at the end of the function that initiates the request, which is before the AJAX request has finished.

You need to move that line into your AJAX success handler.

function loadFile() {
    var xhttp = new XMLHttpRequest();
    var x;
    // The onreadystatechange callback function will execute
    // at some future point after loadFile has completed, so
    // you can only gain access to the AJAX result from within
    // that function.
    xhttp.onreadystatechange = function() {
      if (this.readyState == 4 && this.status == 200) {
        x = this.responseText;
        parseResponse(x);
        return x;
      }
    }
    xhttp.open("GET", "wordList.csv", true);
    xhttp.send();   
}
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • Thank you for your help. After adding that and actually calling on loadFile(); to do its' thing, it's populating my output with "undefined" mixed in. It doesn't seem like I can use loadFile(); to pass the value back to the generate() function and assign that value to holdPass, which carries the passwords as it is built. So it's good news/bad news. Thank you once again for your help! – johnfuller001 Aug 02 '17 at 01:14
0

Your xhttp request is opened asynchronously with the true in xhttp.open("GET", "wordList.csv", true); so x is returned before a value is assigned to it.

Edit: said synchronously instead of asynchronously because I am dumb