The Web Design Group

... Making the Web accessible to all.

Welcome Guest ( Log In | Register )

 
Reply to this topicStart new topic
> Need help with a mysterious problem
CharlesEF
post Feb 22 2023, 08:13 PM
Post #1


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



Hi All,

I call this a mysterious problem because I'm baffled. Let me give you some background. I've been asked to update some old code to allow images to be posted with records that have no type of record code. Ex: ISO countries all have 2 or 3 character alpha codes (US, MX... etc) and a 3 digit numeric code. I still store all images on the HD, not in the DB. The actual path is in the app itself, depending on the record type. I only store the file name in the DB.

I decided to store images based on the record id. When a new record is added and images are uploaded for it, I create a temporary directory to hold the images, since a new record has no id yet (to be renamed when the record is saved). If the user clicks the CANCEL button or changes to a different page without saving the new record I need to delete the temporary directory. Sounds easy enough. To accomplish this I added the else portion to an existing function.

CODE
function checkLoginStatus($post)
{
if(!isset($_SESSION["user"]["id"]))
{
  addErrorMessage("login", "You must log in before you can access our database.");
  redirectToURL("/appl/");
} // Closing brace for if(!isset($_SESSION["user"]["id"]))
else
{
echo("post: {$post}" . PHP_EOL);
  if((bool)$post && !empty($_SESSION["user"]["upload"]["bpath"]) && !empty($_SESSION["user"]["upload"]["newdir"]))
  {
   if(file_exists("{$_SERVER['DOCUMENT_ROOT']}{$_SESSION['user']['upload']['bpath']}/{$_SESSION['user']['upload']['newdir']}"))
   {
echo("delete" . PHP_EOL);
    deleteDirectory("{$_SERVER['DOCUMENT_ROOT']}{$_SESSION['user']['upload']['bpath']}/{$_SESSION['user']['upload']['newdir']}");
   } // Closing brace for if(file_exists("{$_SERVER['DOCUMENT_ROOT']}{$_SESSION['user']['upload']['bpath']}/{$_SESSION['user']['upload']['newdir']}"))
   unset($_SESSION["user"]["upload"]);
  } // Closing brace for if((bool)$post && !empty($_SESSION["user"]["upload"]["bpath"]) && !empty($_SESSION["user"]["upload"]["newdir"]))
} // Closing brace for if(!isset($_SESSION["user"]["id"])) else
}

The first echo shows me the value passed in to the function. The 2nd echo shows 'delete' when the directory is removed.

CODE
checkLoginStatus(empty($_POST) ? 1 : 0);

This is how the function is called. If the $_POST array is empty then a 1 is passed. Otherwise a 0 is passed. This handles the problem when CANCEL is clicked or the page is changed without saving the new record.

Now the problem I found.
When a new record is saved and both client and server side validation both pass but MySQL reports on error (say duplicate key). I see 'post: 0' with no 'delete' but the actual temporary directory is removed. If I comment out the 'deleteDirectory' line in the function and run the test again I see 'post: 0' with no 'delete' and the temporary directory is still there.

How is the 'else' portion being run when 0 is passed to the function?

If you need any more info just ask.

This post has been edited by CharlesEF: Feb 22 2023, 08:16 PM
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Feb 24 2023, 05:25 PM
Post #2


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



One thing I should have mentioned before. I had an else portion for 'if((bool)$post && .....)' and it echoed 'skipped'. So, the logic appears to be correct. Why does commenting out the 'deleteDirectory' command in the function fix the problem? According to the logic it never executes.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
Christian J
post Feb 24 2023, 07:07 PM
Post #3


.
********

Group: WDG Moderators
Posts: 9,628
Joined: 10-August 06
Member No.: 7



Haven't done PHP for a long time, but is this part correct:
CODE
if((bool)$post

? Searching the PHP manual only returns things like is_bool() and boolval(). unsure.gif


User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Feb 25 2023, 03:38 AM
Post #4


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



QUOTE(Christian J @ Feb 24 2023, 06:07 PM) *

Haven't done PHP for a long time, but is this part correct:
CODE
if((bool)$post

? Searching the PHP manual only returns things like is_bool() and boolval(). unsure.gif
As far as I know (bool) is a valid way to cast a value. I will admit it was a recent addition, the 0 or 1 without it should work fine. I've found the point in the code that causes the problem, it was not the function. After some sleep I'll try to figure out why.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Feb 27 2023, 03:41 PM
Post #5


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



I must admit I'm still baffled but let me report what I've found.

The problem was not in the PHP code, it was in the HTML markup. This line:
CODE
<img id="iflag" class="<?php echo((!empty($_POST["id"]) && !empty($_POST["flag"]) && empty($validator->errors["flag"])) ? "img" : "hide");?>" style="border: 1px solid #000000; width: 75%;" src="<?php echo(!empty($_POST["flag"]) ? "{$_POST['bpath']}/" . (!empty($_SESSION["user"]["upload"]["newdir"]) ? $_SESSION["user"]["upload"]["newdir"] : $_POST["id"]) . "/{$_POST['flag']}" : "#");?>" alt="<?php if(!empty($_POST["cname"])) echo("{$_POST["cname"]} flag - front");?>">

Thee are 8 flag name inputs and 8 flag img's to show the flag. If no flag name is set then it is hidden. If I fill in all 8 flag names and submit then the temp directory is not deleted. If I fill in only 1 flag name then the temp directory is deleted. If I comment out the 'deleteDirectory' command in the function itself then the temp directory is not deleted. The function logic has shown that the 'deleteDirectory' section is skipped so why? Commenting out the command is not a valid option. This brings me back to the img tag.

If a flag name is given the markup is:
CODE
<img id="iflag" class="img" style="border: 1px solid #000000; width: 75%;" src="/appl_files/images/flags/2508834891/flag.png" alt="Testing flag - front">

If no flag name is given then the markup is:
CODE
<img id="iflagr" class="hide" style="border: 1px solid #000000; width: 75%;" src="#" alt="Testing flag - reverse">


Is there something special about the img tag I don't know about?
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
Christian J
post Feb 27 2023, 04:18 PM
Post #6


.
********

Group: WDG Moderators
Posts: 9,628
Joined: 10-August 06
Member No.: 7



QUOTE(CharlesEF @ Feb 27 2023, 09:41 PM) *

If I fill in all 8 flag names and submit then the temp directory is not deleted. If I fill in only 1 flag name then the temp directory is deleted.

That sounds more like it's the flag form fields that are the cause, not the images? What happens if you fill on 2 to 7 flag names?
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Feb 27 2023, 07:31 PM
Post #7


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



QUOTE(Christian J @ Feb 27 2023, 03:18 PM) *

QUOTE(CharlesEF @ Feb 27 2023, 09:41 PM) *

If I fill in all 8 flag names and submit then the temp directory is not deleted. If I fill in only 1 flag name then the temp directory is deleted.

That sounds more like it's the flag form fields that are the cause, not the images? What happens if you fill on 2 to 7 flag names?

If I fill in any less than 8 and submit, to get the MySQL error: duplicate key, then the temp directory is deleted. If I fill in 1, 2, 3, 4, 5, 6 or 7 flag names then the temp directory is deleted. The page redraws showing all the correct information except the flag images, because the temp directory is gone.

This is the flag name markup:
CODE
    <div class="container" title="Please enter the country flag file name.">
     <label for="flag">Flag:</label><br>
     <input type="text" id="flag" name="flag" maxlength="50" onchange="java script:doesImageExist(this, '<?php echo($help_name);?>', '<?php echo(generateToken("$help_name " . $phrases[$_SESSION[$help_name]["phrase"]] . " $help_name", $secrets[$_SESSION[$help_name]["secret"]], false));?>');" value="<?php if(isset($_POST["flag"])) echo($_POST["flag"]);?>"><br>
     <span id="cefiform_flag_errorloc" class="error"><?php if(isset($validator->errors["flag"])) echo(implode("<br>", $validator->errors["flag"]));?></span>
    </div>


I don't see how any HTML markup could cause the temp directory to be deleted.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
Christian J
post Feb 27 2023, 08:28 PM
Post #8


.
********

Group: WDG Moderators
Posts: 9,628
Joined: 10-August 06
Member No.: 7



Do you mean you have eight identical INPUT elements like this in the same form:

CODE
<input type="text" id="flag" name="flag"

--all with the same ID and NAME value? unsure.gif
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Feb 28 2023, 02:59 PM
Post #9


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



QUOTE(Christian J @ Feb 27 2023, 07:28 PM) *

Do you mean you have eight identical INPUT elements like this in the same form:

CODE
<input type="text" id="flag" name="flag"

--all with the same ID and NAME value? unsure.gif

No, they all have different ID and NAME values. I only showed the first.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
Jason Knight
post Mar 2 2023, 12:27 PM
Post #10


Advanced Member
****

Group: Members
Posts: 100
Joined: 25-December 22
Member No.: 28,719



I have more questions than I can provide answers...

empty() returns a perfectly good boolean result, so why are you turning it into 0 and 1 just to turn it back into boolean?!?

$_POST is a superglobal, so why bother passing if it's empty or not as a parameter?

Why are you triple escaping your echo? ("{ Really?

I think a lot of your issues could be cleaned up with better formatting, leveraging variables to not repeat yourself so much, removing variables you don't need, and just in general writing cleaner PHP than... whatever that is.

CODE

function checkLoginStatus() {

    if(!isset($_SESSION['user']['id'])) {
        addErrorMessage('login', 'You must log in before you can access our database.');
        redirectToURL('/appl/');
        return;
    }
    
    if (
        empty($_POST) &&
        !empty($_SESSION['user']['upload']['bpath']) &&
        !empty($_SESSION['user']['upload']['newdir'])
    ) {
        $file =
            $_SERVER['DOCUMENT_ROOT'] .
            $_SESSION['user']['upload']['bpath'] . '/' .
            $_SESSION['user']['upload']['newdir'];
        if (is_directory($file) {
            echo 'delete', PHP_EOL;
            deleteDirectory($file);
        }
    }
    
    unset($_SESSION['user']['upload']);
    
} // checkLoginStatus


A little "premature exit" can really help smooth out if/else ambiguities and simplify the code.

CODE

<?php
    echo '
        <img
            id="iflag"
            class="', (
                !empty($_POST["id"]) &&
                !empty($_POST["flag"]) &&
                empty($validator->errors["flag"])
            ) ? 'img' : 'hide', '
            src="', (
                empty($_POST["flag"]) ?
                '#' :
                $_POST['bpath'] . '/' .  (
                    !empty($_SESSION['user']['upload']['newdir']) ?
                    $_SESSION['user']['upload']['newdir'] :
                    $_POST['id'] . '/' . $_POST['flag']
                )
            ), '"', (
                empty($_POST["cname"]) ?
                '' : '
            alt="', $_POST["cname"], ' flag - front"'
            ), '
        >';


Your input markup is equally confusing, since you have TITLE on a DIV (for what exactly), possibly a DIV for nothing, "java script" instead of JavaScript for something that shouldn't even be in the markup in the first place...

See this nightmare:
CODE

    onchange="doesImageExist(this, '<?php echo($help_name);?>', '<?php echo(generateToken("$help_name " . $phrases[$_SESSION[$help_name]["phrase"]] . " $help_name", $secrets[$_SESSION[$help_name]["secret"]], false));?>');"

A big helpful tip, in any modern (v7/newer) version of PHP <?= $whatever ?> is equivalent to <?php echo $whatever; ?>

But more importantly, if you're using onevent attributes you're doing it all wrong. It means your function needs to be in the global scope, it means you're missing caching opportunities... quality scripting should enhance the already working form and hook the markup remotely, not be slopped into the markup!

Thus:
CODE

<?php

    echo '
    <label>
        Flag:<br>
        <input
            name="flag"
            maxlength="50"
            data-name="', $help_name, '"
            data-token="', generateToken(
                $help_name . ' ' .
                $phrases[$_SESSION[$help_name]['phrase']] . ' ' .
                $help_name,
                $secrets[$_SESSION[$help_name]["secret"]],
                false
            ), '"';
            
    if (!empty($_POST["flag"])) echo '
            value="', htmlspecialchars($_POST['flag']), '"';
            
    echo '
        ><br>
        <span id="cefiform_flag_errorloc" class="error">';
  
    if (isset($validator->errors["flag"])) {
        echo implode("<br>", $validator->errors["flag"]);
    }
    
    echo '</span><br>
    </label>';


Then have the client-side scripting apply the event handler like a good little doobie. Though I'm guessing wildly here as I think you're taking something simple and overcomplicating it.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Mar 3 2023, 06:25 PM
Post #11


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



Hi Jason,

I agree with some of your criticisms but not all. And I will admit that when I'm in programming mode my brain works faster than my fingers can type, so the 1st draft might not be as efficient as it can be. But, I ALWAYS review the code to optimize it and sometimes come up with a new way.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post
CharlesEF
post Mar 28 2023, 02:02 PM
Post #12


Programming Fanatic
********

Group: Members
Posts: 1,981
Joined: 27-April 13
From: Edinburg, Texas
Member No.: 19,088



I was never able to figure out the actual problem. I did come up with a work around. I removed all 'img' tags and use PHP or JS to add them when needed.
User is offlinePM
Go to the top of the page
Toggle Multi-post QuotingQuote Post

Reply to this topicStart new topic
2 User(s) are reading this topic (2 Guests and 0 Anonymous Users)
0 Members:

 



- Lo-Fi Version Time is now: 18th March 2024 - 10:45 PM