Document Dispatch not creating patient folder

Day-to-Day Users
#1

I have a strange problem in 5.0.1 (6)
When I go to Fax/Scan and select a fax or a scanned document to upload to a patients chart, the document saves in documents root folder;
/openemr/sites/default/documents/
instead of
/openemr /sites/default/document/214 (where 214 is the PID)
and the URL inserted into the documents table has a double forward slash and no PID. The documents pull up fine in the patients chart since the URL works but my
/openemr/sites/default/documents folder is completely disorganized.

Any ideas on how to fix this in fax_dispatch.php ?

file:///var/www/openemr/sites/default/documents//test.pdf

#2

Hi, i’ll take a quick look at this. I can’t imagine why this hasn’t come up before unless folks don’t realize it is broken. What’s the chances huh :slight_smile:

#3

cool ! tbanks for looking into this.
I am wondering if this problem is tied into the issue of not being able to rename the file to be dispatched. Not being able to rename the file is a major nuisance when dispatching faxes especially.

#4

Unsure about rename but first do any fax pdf saves ever get to your document/pid directories?

#5

No Jerry none of the faxes or scan get to documents/pid folder.
Even if the pid folder exists, the fax does not get inserted into the pid folder.

#6

Okay so looking at code when the file path is built the first test is if a pid is present and then is sanitized. So to fall through to get saved without a pid is odd. Workflow is to throw an exception or die.
This troubles me exec("mkdir -p " . escapeshellarg($docdir)); still I can’t immediately see how the file path can get built w/o the pid and not die.
You might try changing the conditional if (!$patient_id) around L118 to maybe `if ($patient_id < 1)’ or not cast the $patient_id to (int) in a prior statement. Still it should work as written but I can’t test as I don’t have halifax set up.
Do you have an ide w/ debug you can step through code to see how path is built? Or you could add a die after the path is built like die(‘Here is my path:’ . $docdir); to just verify that path…

#7

Thanks Jerry
I will test out.
You don’t need hylafax to test.
The same errant behavior can be observed in the “Scanner In” section of Scan/Fax tab.

#8

I don’t know what this means.

#9

OK it seems fixed; I followed your suggestions and looked at those lines and asked the die after returning the path.
The PID was not being returned. So I took the function check_file_dir_name out and now the files save as expected in the pid folder!

Line 111

$docdir = $GLOBALS[‘OE_SITE_DIR’] . “/documents/” . check_file_dir_name($patient_id);

Change to
Line 111

$docdir = $GLOBALS[‘OE_SITE_DIR’] . “/documents/” .$patient_id;

I dont seem to have broken anything.
The documents save in the correct PID folder if folder exists and if not, it creates a pid folder and puts document in it.

#10

Additionally, I took out check_file_dir_name from
line 122

$ffname = check_file_dir_name(trim($_POST[‘form_filename’]);

and changed it to

$ffname = trim($_POST[‘form_filename’]);

and now filename changes are sticking!

Change Filename on the Fax Dispatch page
#11

Makes sense but I think the real fix is with where the $patient_id is type cast to an integer. Windows won’t mind much but linux may not like passing an int around however I am a bit surprised the validation didn’t die! This works as written on my windows machine so draw from that what you will.
Would be very curious if you go back to original code and took out the casting to int of patient_id if that is indeed the true fix…

#12

Can you show me exactly what I need to take out?
Which line etc
Ill be happy to try it.

#13

L105 $patient_id = (int) $_POST[‘form_pid’]; take away the (int).
Also note all of this is broken in v502 due to something amiss with new verifyCsrfToken security modifications. So there is that!

#14

Thanks.
Yes, I tested in 5.02 and got a “Authentication Failed” error.

#15

Hi,

This should fix the authentication error:
fax_dispatch.php bug fix by bradymiller · Pull Request #2239 · openemr/openemr · GitHub

The check_file_dir_name() was fixed in development codebase to return the parameter:
openemr/sanitize.inc.php at master · openemr/openemr · GitHub

Whereas in 5.0.1 this is not the case:
openemr/sanitize.inc.php at rel-501 · openemr/openemr · GitHub
(it doesn’t return anything when it passes the test)

-brady

1 Like
#16

Thanks!!
That worked.
:+1:

#17

Kept looking at that function thinking there is nothing wrong with the validation. Forest, trees etc. thanks @brady.miller

#18

Hi Brady,
I went to fix my dev version of 501 and this fix is already there so either I back ported and forgot to put in patch 7 or i’m sleep coding again. :slight_smile:
Does/should this need to go in Patch 8? @brady.miller

#19

@sjpadgett , Yep,
Not sure why I didn’t flag that fix on my end either(guessing I was running low on coffee :slight_smile: ). Agree to bring fix into patch 8.
-brady