Ticket #3058 (closed Bugs: fixed)

Opened 5 years ago

Last modified 8 months ago

System recordings - Playback Reporting wrong MIME Type

Reported by: lostdave Assigned to:
Priority: minor Milestone: 2.5
Component: System Recordings Version: 2.5-branch
Keywords: Cc:
Confirmation: Need testing Distro:
Backend Engine: Asterisk 1.4.x Distro Ver:
Backend Ver: SVN Revision (if applicable):

Description

Version 2.50Beta1.2 If you press the Play Button next to a recording, Firefox 2.0.0.16 complains about a missing plugin. Upon trying to find a Plugin it reports that it was unable to find a plugin Unknown Plugin(text/html)

Attachments

something-terribly-wrong.gsm (2.8 kB) - added by lostdave on 08/15/08 16:45:07.
System Message
test_message.wav (101.8 kB) - added by lostdave on 08/15/08 16:45:44.
Test messages recorded with system recordings using *77
url.JPG (20.1 kB) - added by mickecarlsson on 08/15/08 17:57:06.
embed with '#' at the end

Change History

08/14/08 10:20:52 changed by p_lindheimer

  • confirmation changed from Unreviewed to Need Feedback.

I previously had this same issue until the changes we made that addressed the access violations and fixed the encryption to be a bit more proper. Any chance you had an issue upgrading properly? Can you make sure all the key php files in /var/www/html/admin/modules/recordings are in fact the correct ones from the module (check svn to make sure you are completely up-to-date). From what I've seen that usually happens when you are having mis-matches with the encryption that is going on.

08/14/08 10:50:23 changed by mickecarlsson

It is because it is missing the extension of the file. The file is displayed in the field without suffix and when you press p.la play it will take the file name, but as it is without proper extension (for example .wav) it will not work.

08/14/08 11:06:16 changed by p_lindheimer

? you mean someone is trying to play something like a gsm with media player that does not support that format? So I should close the ticket (since quicktime plays all the formats that I have loaded anyhow)

08/14/08 11:34:37 changed by mickecarlsson

No, what I actually mean is that the variable in php containing the file name for the audio file is without the .wav (or whatever the file extension is) extension. And when the call is made to popup and play the file it is missing a proper file name. For example, the recording module lists a custom file custom/TestThis, the file is actually custom/TestThis.wav.

The code for this is here (popup.php)

        $REC_CRYPT_PASSWORD = (isset($amp_conf['AMPPLAYKEY']) && trim($amp_conf['AMPPLAYKEY']) != "")?trim($amp_conf['AMPPLAYKEY']):'moufdsuu3nma0';
  $path = $crypt->decrypt($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD);
  $file = $crypt->encrypt($path.$_REQUEST['recording'],$REC_CRYPT_PASSWORD);
  $ufile = $_REQUEST['recording'];

$ufile is the variable for the file, but it lacks extension. I have tried to do a quick test with:

  $ufile = $_REQUEST['recording'];
  $ufile = $ufile.".wav";

But it still fail, but now it has the extension.

08/14/08 12:00:54 changed by p_lindheimer

I don't think that is it, the extension is put back when passed to audio.php:

  $opath = $_REQUEST['recording'];
  $path = $crypt->decrypt($opath,$REC_CRYPT_PASSWORD);

  // strip ".." from path for security
  $path = preg_replace('/\.\./','',$path);

  // See if the file exists, otherwise check for extensions
  if (is_file("$path.wav")) { $path="$path.wav"; }
  elseif (is_file("$path.Wav")) { $path="$path.Wav"; }
  elseif (is_file("$path.WAV")) { $path="$path.WAV"; }
  elseif (is_file("$path.mp3")) { $path="$path.mp3"; }
  elseif (is_file("$path.gsm")) { $path="$path.gsm"; }
  elseif (!is_file($path)) { die("<b>404 File not found!: $opath </b>"); }

  // Gather relevent info about file
  $size = filesize($path);
  $name = basename($path);
  $extension = strtolower(substr(strrchr($name,"."),1));

  // This will set the Content-Type to the appropriate setting for the file
  $ctype ='';
  switch( $extension ) {
    case "mp3": $ctype="audio/mpeg"; break;
    case "wav": $ctype="audio/x-wav"; break;                                                                                                                    
    case "Wav": $ctype="audio/x-wav"; break;                                                                                                                    
    case "WAV": $ctype="audio/x-wav"; break;                                                                                                                    
    case "gsm": $ctype="audio/x-gsm"; break;                                                                                                                    

followed eventually by:

  // need to check if file is mislabeled or a liar.                                                                                                             
  $fp=fopen($path, "rb");                                                                                                                                       
  if ($size && $ctype && $fp) {                                                                                                                                 
    header("Pragma: public");                                                                                                                                   
    header("Expires: 0");                                                                                                                                       
    header("Cache-Control: must-revalidate, post-check=0, pre-check=0");                                                                                        
    header("Cache-Control: public");                                                                                                                            
    header("Content-Description: wav file");                                                                                                                    
    header("Content-Type: " . $ctype);                                                                                                                          
    header("Content-Disposition: attachment; filename=" . $name);                                                                                               
    header("Content-Transfer-Encoding: binary");                                                                                                                
    header("Content-length: " . $size);                                                                                                                         
    fpassthru($fp);                                                                                                                                             
  }                                                                                                                                                             

maybe we need to check for more extensions or the 404 not found is not done properly and resulting in the described issue. (So maybe we need to make the check of acceptable formats in popup.php and report the error there, as well as add the extension of the chose path there so when we pass it it just plays it... thoughts?

08/14/08 13:49:20 changed by p_lindheimer

ok - just did a quick check by creating a g729 bogus file and get that, I'll try to have a look at changing the above as I suggest so the popup is modified if it finds a format it does not think it can play. I'm pretty sure the 404 needs to be moved into the popup.php - it confused the embed directive otherwise and you never see the error.

08/14/08 14:50:09 changed by mickecarlsson

I cant figure this one out if it is my install or if it is another issue.

If I add this line just below the <?php I see it whenever I click on play file:

  echo "Inside crypt.php";

If I add this line just below the <?php in audio.php I never see the text.

  echo "Inside audio.php";

To me it sounds like audio.php never gets executed.

08/14/08 18:15:32 changed by lostdave

I now have 3 systems all with the previously described behaviour. I have checked and diffed the module against SVN and it is the latest.

all the files have a .wav extension and if I transfer them back to a PC, they play fine, but I see the same as mickecarlsson, No Filename Extension is being parsed. I will have more of a dig into this.

08/14/08 23:19:03 changed by p_lindheimer

yes no file extension is parsed, the code in audio.php looks for he file extension as mentioned (see snapshot above). But I'm going to try moving that up to popup.php since printing anything in audio.php will result in garbage, it's trying to interpret it as part of the embedded audio player function.

08/15/08 00:44:59 changed by p_lindheimer

Try the following patches. Now when I have a format other than wav, gsm or mp3 it provides a printed error. Question, what other of the formats should common plugin players be able to play and what is the correct Content Type (e.g. audio/x-wav)

Index: audio.php
===================================================================
--- audio.php   (revision 6374)
+++ audio.php   (working copy)
@@ -16,17 +16,6 @@
   $opath = $_REQUEST['recording'];
   $path = $crypt->decrypt($opath,$REC_CRYPT_PASSWORD);
 
-  // strip ".." from path for security
-  $path = preg_replace('/\.\./','',$path);
-  
-  // See if the file exists, otherwise check for extensions
-  if (is_file("$path.wav")) { $path="$path.wav"; }
-  elseif (is_file("$path.Wav")) { $path="$path.Wav"; }
-  elseif (is_file("$path.WAV")) { $path="$path.WAV"; }
-  elseif (is_file("$path.mp3")) { $path="$path.mp3"; }
-  elseif (is_file("$path.gsm")) { $path="$path.gsm"; }
-  elseif (!is_file($path)) { die("<b>404 File not found!: $opath </b>"); }
-
   // Gather relevent info about file
   $size = filesize($path);
   $name = basename($path);

and

Index: popup.php
===================================================================
--- popup.php   (revision 6374)
+++ popup.php   (working copy)
@@ -33,10 +33,25 @@
 
        $REC_CRYPT_PASSWORD = (isset($amp_conf['AMPPLAYKEY']) && trim($amp_conf['AMPPLAYKEY']) != "")?trim($amp_conf['AMPPLAYKEY']):'moufdsuu3nma0';
 
-  $path = $crypt->decrypt($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD);
-  $file = $crypt->encrypt($path.$_REQUEST['recording'],$REC_CRYPT_PASSWORD);
-  $ufile = $_REQUEST['recording'];
+  $path = $crypt->decrypt($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD).$_REQUEST['recording'];
 
+  // strip ".." from path for security
+  $path = preg_replace('/\.\./','',$path);
+       $ufile = basename($path);
+  
+  // See if the file exists, otherwise check for extensions
+  if (is_file("$path.wav")) { $path="$path.wav"; }
+  elseif (is_file("$path.Wav")) { $path="$path.Wav"; }
+  elseif (is_file("$path.WAV")) { $path="$path.WAV"; }
+  elseif (is_file("$path.mp3")) { $path="$path.mp3"; }
+  elseif (is_file("$path.gsm")) { $path="$path.gsm"; }
+  else {
+               echo("<br /><h1 class='popup_download'>".sprintf(_("No compatible wav, mp3 or gsm format found to play:<br /><br />%s"),$ufile)."</h1><br>");
+               exit;
+       }
+
+  $file = $crypt->encrypt($path,$REC_CRYPT_PASSWORD);
+
   if (isset($file)) {
     echo("<br>");
     echo("<embed src='".$_SERVER['PHP_SELF']."?display=recordings&action=audio&recording=$file' width=300, height=20 autoplay=true loop=false></embed><br>");

08/15/08 03:00:31 changed by lostdave

Okay ran the Patches over SVN 6401

3 Differenct Scenarios

1. System Recording done when this box was @ 2.3 Result: Error Message "No compatible wav, mp3 or gsm format found to play:" But the file is present and exists and still plays happily on the 2.3 box

2. Newly record file via *77 Result: Missing Plugin Error from FF(2 and 3) Again - Can use the file in IVR's and is available by the feature code Option

3. System File assigned to recording Result - Same as 2 with obvious exception of allowing editing via the Feature code

08/15/08 10:13:28 changed by p_lindheimer

what is the exact name of the file in case (1) complete with the extension as it may indicate that we are not checking all proper types. (as it is only approving wav, Wav, WAV, gsm and mp3 right now). In the case of (2) and (3), please attach those files to the ticket so we can take a look.

08/15/08 11:40:21 changed by mickecarlsson

I have tested this with IE6 and FF 3.0, I have 5 files with .wav extension in custom/, I always get a No compatible wav, mp3 or gsm format found to play:. But, sometimes the embed kicks in (with quicktime) and display something, but the file does not play.

I the selected a built-in recording, clicked on Play and got this:

No compatible wav, mp3 or gsm format found to play:

asterisWl��.�x,�Kr�customer-service

Could this be a memory issue with php as all files in sound are in the html-output as:

<option value=""></option>
<option value=".asterisk-core-sounds-en-gsm-1.4.9">.asterisk-core-sounds-en-gsm-1.4.9</option>

<option value="1-for-am-2-for-pm">1-for-am-2-for-pm</option>
<option value="1-yes-2-no">1-yes-2-no</option>
<option value="CHANGES-asterisk-core-en-1.4.9">CHANGES-asterisk-core-en-1.4.9</option>
<option value="CREDITS-asterisk-core-en-1.4.9">CREDITS-asterisk-core-en-1.4.9</option>
<option value="T-changed-to">T-changed-to</option>
..
rest of <option value....

If a view the page as source and then save it, it is 317 kB (325 234 byte).

08/15/08 12:59:16 changed by p_lindheimer

mickecarlsson, I would not thing it is a memory issue. However, as my previous request to dave, can you provide the exact filename that gives you the "No compatible wav..." message. As you can see, the code is very deterministic so that part of it we should be able to troubleshoot. If is specifically looking for the filename and reporting that if it does not get it.

As far as showing up in the select list, that lists considers every single file in the directory (even bogus files that are not sound files...)

08/15/08 15:47:41 changed by mickecarlsson

Here they are:

-rwxrwxr-x 1 asterisk asterisk  91404 Aug 15 18:41 AddproOpen1.gsm
-rwxrwxr-x 1 asterisk asterisk  91404 Aug 15 18:39 AddproOpen.gsm
-rwxrwxr-x 1 asterisk asterisk  91404 Aug 14 19:22 AddproOpen.wav
-rwxrwxr-x 1 asterisk asterisk 219724 Aug 14 19:22 JulStangt.wav
-rwxrwxr-x 1 asterisk asterisk  61324 Aug 14 19:22 MangaRinger.wav
-rwxrwxr-x 1 asterisk asterisk 122444 Aug 14 19:22 SupportenOpen.wav
-rwxrwxr-x 1 asterisk asterisk 224204 Aug 14 19:22 Supporten.wav

One thing I did was to change the code a bit to provide some debugging:

root@pbx:/var/www/html/admin/modules/recordings $ diff popup.php.org popup.php
40c40,41
<        $ufile = basename($path);
---
> //       $ufile = basename($path);
>        $ufile = $path;

And now I can see some problematic issues: When I get the plugin not found in Firefox the file name printed to play is:

playing: /var/lib/asterisk/sounds/custom/AddproOpen

If I continue to click play my result are as above. If I then select another custom recording I get this:

No compatible wav, mp3 or gsm format found to play:

/var/lib/astervn��Ͷj�X{"C맃�custom/AddproOpen1

If I select another file I get this result:

No compatible wav, mp3 or gsm format found to play:

/var/lib/aster��4�+THQL�V`t�gBcustom/SupportenOpen

or:

No compatible wav, mp3 or gsm format found to play:

��YB[(��v�+j"�`�0�%���&�custom/JulStangt

However, sometimes I get an almost perfect result, it says, file name and path is OK, QuickTime? kicks in but is dimmed and no sound is played.

I suspect that the crypt function is to blame or some other things that corrupt the path.

08/15/08 16:44:21 changed by lostdave

OK...

My custom/ DIR List

[root@pbx custom]# ls -al
total 2284
drwxrwxr-x  2 asterisk asterisk   4096 Aug 15 16:50 .
drwxrwxr-x 15 asterisk asterisk  73728 Aug 13 12:27 ..
-rw-rw-r--  1 asterisk asterisk 163084 Mar 17  2006 aa_1.wav
-rw-rw-r--  1 asterisk asterisk 130444 Nov 29  2006 alarm1.wav
-rw-rw-r--  1 asterisk asterisk 269964 Sep  7  2006 Axiom_AH.wav
-rw-rw-r--  1 asterisk asterisk  94284 Sep  8  2006 Axiom_Day.wav
-rw-rw-r--  1 asterisk asterisk  61324 Sep  7  2006 Conf.wav
-rw-rw-r--  1 asterisk asterisk 157964 Sep  7  2006 dayswitch.wav
-rw-rw-r--  1 asterisk asterisk 286924 Apr  2 17:32 DCP_AH.wav
-rw-rw-r--  1 asterisk asterisk 122444 Mar 13  2006 Main_Queue.wav
-rw-rw-r--  1 asterisk asterisk 149964 Sep  7  2006 niteswitch.wav
-rw-rw-r--  1 asterisk asterisk 104524 Aug 29  2006 nor1.wav
-rw-rw-r--  1 asterisk asterisk 251084 Sep  7  2006 Norc_AH.wav
-rw-rw-r--  1 asterisk asterisk  44684 Sep  8  2006 Norcall.wav
-rw-rw-r--  1 asterisk asterisk  98124 Mar 16  2006 TC_Main.wav
-rw--w----  1 asterisk asterisk 104204 Aug 15 16:50 test_message.wav
-rw-rw-r--  1 asterisk asterisk 139084 Aug 29  2006 wait.wav

The New message recorded was test_message and the inbuilt that I am using is also attached

08/15/08 16:45:07 changed by lostdave

  • attachment something-terribly-wrong.gsm added.

System Message

08/15/08 16:45:44 changed by lostdave

  • attachment test_message.wav added.

Test messages recorded with system recordings using *77

08/15/08 16:53:10 changed by p_lindheimer

do you have AMPLAYKEY set in amportal.conf or are you using the default. In my testing, I do not have it set. If you do, does it fix things if you remove it? Also, it should not be necessary but what happens if you put global $amp_conf; at the top of both audio.php and popup.php ?

lostdave, thanks for the attachments, I'll have a look a bit later.

08/15/08 16:57:31 changed by lostdave

Not set here either...

I will test and report back

Enjoy my sexy voice :-D

08/15/08 17:05:27 changed by mickecarlsson

I have done some serious debugging and have come to the conclusion that when I get scrambled paths is when the base64 decode is returning less characters than it should.

In crypt.php the strlen of the path (when a file selected) when it is fed to encrypt is 25 characters. After the encryption the strlen is 48 characters. Then it is base64 encoded.

When the decrypt routine is called and base64 is decoded the strlen is either 47 characters and you get garbage in the path, or it is 48 characters and you get a plugin error in Firefox.

08/15/08 17:57:06 changed by mickecarlsson

  • attachment url.JPG added.

embed with '#' at the end

08/15/08 17:57:25 changed by lostdave

OK...I did some digging as well, and the only change to the crypt.php side of things for quite some time has been the following

--- modules/branches/2.5/recordings/crypt.php (revision 4220)
+++ modules/branches/2.5/recordings/crypt.php (revision 6234)
@@ -63,5 +63,5 @@
   function decrypt($enc, $salt, $iv_len = 16) {
 
-     $enc = urldecode(base64_decode($enc));
+     $enc = base64_decode(urldecode($enc));
      $n = strlen($enc);
      $i = $iv_len;

as soon as I reverse that patch I stop getting the mangled file names.

Now the only issue is popup.php not actually finding the file...

More testing to follow.

08/15/08 17:59:14 changed by mickecarlsson

See added url.jpg, there is an added '#' at the end of the link for the embed. Should it be there? Or where is that added?

08/15/08 18:04:43 changed by mickecarlsson

Ok, after changing crypt.php I also never get scrambled paths/filenames so we nailed that one. Lets see if we can get the other...

08/15/08 18:32:07 changed by p_lindheimer

ok, I can revert the crypt.php change and all continues to work fine on my side, so that maybe partially to blame:

svn merge -r 6234:6233 crypt.php

concerning the extra '#' I don't see that on my end. It comes from this chunk of code:

  $html_txt .=  "<a href='#' ".(($count)?$hidden_state:'')." type='submit' id='play$count' onClick=\"javascript:popUp('$recurl',document.prompt.sysrec$count); return false;\" input='foo'>";

followed by the javascript below and I don't see either of them introducing the '#'

  function popUp(URL,optionId) {
    var selIndex=optionId.selectedIndex
    var file=optionId.options[selIndex].value

    /*alert(selIndex);*/
    if (file != "")
      popup = window.open(URL+file, 'play', 'toolbar=0,scrollbars=0,location=0,statusbar=0,menubar=0,resizable=1,width=320,height=110');
  }

08/15/08 19:02:21 changed by lostdave

I think we have a problem still with Crypt....

[Sat Aug 16 08:58:14 2008] [error] [client 192.168.16.31] PHP Warning:  filesize() [<a href='function.filesize'>function.filesize</a>]: stat failed for \xd9\x1ft\xa3Qh\t\xf1\xfb\x8cm\xb9\xbd)\xb98k/sounds/custom/test_message.wav in /var/www/html/admin/modules/recordings/audio.php on line 20, referer: http://pbx.dave.local/admin/config.php?display=recordings&action=popup&recordingpath=BZYLtXJokOV1oGws%2FQHFZwZrZIONDc7dX0%2BIEm8t9RjASqd76j0bpRqD7QVxOL5I&recording=custom/test_message

08/15/08 19:16:34 changed by lostdave

I reverted back to using the default password (commented out my AMPLAYKEY setting) Cleared Browser cache and things are working again.

Put the AMPPLAYKEY Setting in again , apply conf, clear browser cache and all still works.

Looks like the only culprit here was crypt.php

if mickecarlsson can confirm, then I think this migh tbe able to be closed.

08/15/08 19:29:10 changed by lostdave

I have confirmed on a clean system that the reversion on crypt.php solves this.

08/15/08 19:31:27 changed by p_lindheimer

lostdave, so reverting the crypt function works for you using both the default crypt key and setting your own in amportal.conf? Is that the prognosis?

08/15/08 19:32:54 changed by lostdave

Correct.

It looks like the bulk of the problem debugging this was junk in the browser cache!

08/15/08 21:57:57 changed by p_lindheimer

  • status changed from new to closed.
  • resolution set to fixed.

r6404 #3058 properly display messages for unplayble formats and revert r6234 for crypt.php

08/16/08 15:27:42 changed by p_lindheimer

  • status changed from closed to reopened.
  • confirmation changed from Need Feedback to Need testing.
  • resolution deleted.

with the new crytp.php reversion, I'm getting plug-in errors again. I try to listen to a new sound file, it gives me an error. I go listen to something else that was working and then go back and listen to the file and it works.

When I revert to the previous change, it never gives the errors. I think the previous change was correct. In addition to testing, here is why I think the previous was correct:

  function encrypt($str, $salt, $iv_len = 16) {
  ...
  return urlencode(base64_encode($enc_text));
  function decrypt($enc, $salt, $iv_len = 16) {
                                                                                                                   
  $enc = base64_decode(urldecode($enc));                                                                        
  ...

note that during encryption we base64 encode AND THEN urlencode, so to reverse that we need to first urldecode AND THEN base64 decode. Right? Please provide feedback, I'm inclined to put this back.

This patch puts it to how I think it should be (what was just reverted):

Index: crypt.php
===================================================================
--- crypt.php   (revision 6405)
+++ crypt.php   (working copy)
@@ -62,7 +62,7 @@
    */
   function decrypt($enc, $salt, $iv_len = 16) {
 
-     $enc = urldecode(base64_decode($enc));
+     $enc = base64_decode(urldecode($enc));
      $n = strlen($enc);
      $i = $iv_len;
      $str = '';

08/16/08 18:04:00 changed by lostdave

OK..To Break this down into logical steps

we Have the Following process to Encrpypt

Ecrypted String -> Base 64 Encode = String1 Urlenecode -> String1 = Encrypted Path

To Decrypt we have to do the reverse

Urldecode -> Encrypted Path = String 1 Base 64 Decode -> String 1 = Encrypted String

Which with the reversion is what we are doing. Or is there a flaw in my Logic?

Dave

08/16/08 19:03:32 changed by lostdave

Yet again....I type before I think..... Brackets are done first.....so the reversion of the reversion does follow the logic.. But it doesn't explain the results....

08/16/08 20:06:38 changed by p_lindheimer

too many levels of 'reversion' - I can't follow. To make it simple, do I apply the above patch which is the original change that I had made, and which I got convinced to undo a day ago?

08/16/08 20:40:09 changed by lostdave

If you apply the above patch, then that is the correct order of things..and it should return the correct value

I wrote a little test program that encrypts a string then decrypted it using both methods, and yours is correct BUT when I apply the change to the inplace code, I am back to screwed up output

in the Below, Path 1 is with your latest above patch. Path2 is as per r6405.

Decrypt Path 1 is: /a�W6�a��� �|��k�f��
F:`p�T-to-leave-msg
Decrypt Path 2 is: /var/lib/asterisk/sounds/T-to-leave-msg

my current diff against svn so you can see my test methods

Index: crypt.php
===================================================================
--- crypt.php   (revision 6405)
+++ crypt.php   (working copy)
@@ -62,7 +62,8 @@
    */
   function decrypt($enc, $salt, $iv_len = 16) {

-     $enc = urldecode(base64_decode($enc));
+     //$enc = urldecode(base64_decode($enc));
+     $enc = base64_decode(urldecode($enc));
      $n = strlen($enc);
      $i = $iv_len;
      $str = '';
@@ -75,6 +76,23 @@
      }
      return preg_replace('/\\x13\\x00*$/', '', $str);
   }
+  function decrypt2($enc, $salt, $iv_len = 16) {
+
+     //$enc = urldecode($enc);
+     $enc = base64_decode($enc);
+     $enc = urldecode($enc);
+     $n = strlen($enc);
+     $i = $iv_len;
+     $str = '';
+     $iv = substr($salt ^ substr($enc, 0, $iv_len), 0, 512);
+     while ($i < $n) {
+         $block = substr($enc, $i, 16);
+         $str .= $block ^ pack('H*', md5($iv));
+         $iv = substr($block . $iv, 0, 512) ^ $salt;
+         $i += 16;
+     }
+     return preg_replace('/\\x13\\x00*$/', '', $str);
+  }
 }


Index: popup.php
===================================================================
--- popup.php   (revision 6405)
+++ popup.php   (working copy)
@@ -34,7 +34,11 @@
        $REC_CRYPT_PASSWORD = (isset($amp_conf['AMPPLAYKEY']) && trim($amp_conf['AMPPLAYKEY']) != "")?trim($amp_conf['AMPPLAYKEY']):'moufdsuu3nma0';

   $path = $crypt->decrypt($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD).$_REQUEST['recording'];
-
+  echo("Decrypt Path 1 is: ".$path);
+  echo("<BR>");
+  $path2 = $crypt->decrypt2($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD).$_REQUEST['recording'];
+  echo("Decrypt Path 2 is: ".$path2);
+  echo("<BR>");
   // strip ".." from path for security
   $path = preg_replace('/\.\./','',$path);
        $ufile = basename($path);
@@ -51,6 +55,13 @@
        }

   $file = $crypt->encrypt($path,$REC_CRYPT_PASSWORD);
+  $filetest2 = $crypt->encrypt($path2,$REC_CRYPT_PASSWORD);
+  $decfile1 = $crypt->decrypt($file,$REC_CRYPT_PASSWORD).$_REQUEST['recording'];
+  echo("Decrypt Path 1 is: ".$decfile1);
+  echo("<BR>");
+  $decfile2 = $crypt->decrypt2($file,$REC_CRYPT_PASSWORD).$_REQUEST['recording'];
+  echo("Decrypt Path 2 is: ".$decfile2);
+  echo("<BR>");

   if (isset($file)) {
     echo("<br>");

08/16/08 22:45:21 changed by p_lindheimer

at this point I'm going to apply it back and then we'll have to see what other people are reporting. With it in place, I am not getting any errors.

08/16/08 22:48:17 changed by p_lindheimer

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [6410]) fix #3058 again, hopefully really this time, un-revert crypt.php

08/17/08 10:58:48 changed by mickecarlsson

  • status changed from closed to reopened.
  • resolution deleted.

After almost two hours of debugging I have found out why we get corrupted file names. And why it random.

In crypt.php we encrypt the string in crypt->encrypt and return it

return urlencode(base64_encode($enc_text));

But this is never used in popup.php as we send $_REQUESTrecordingpath? to crypt->decrypt

 $crypt = new Crypt();

        $REC_CRYPT_PASSWORD = (isset($amp_conf['AMPPLAYKEY']) && trim($amp_conf['AMPPLAYKEY']) != "")?trim($amp_conf['AMPPLAYKEY']):'moufdsuu3nma0';

  $path = $crypt->decrypt($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD);

It will work until recordingpath contains a '+' sign (that are changed to %2B in urlencode). But as we send the NOT urlencoded string to crypt-decrypt it will of course fail and produce a bogus file path whenever it hits the '+' sign.

08/17/08 11:18:36 changed by mickecarlsson

NAILED IT!!!!

It is the browser that is doing the urlencoding and that it is why it fails randomly. By this patch this I managed to solve it, it works all the time now.

¨--- popup.php.org       2008-08-16 03:55:58.000000000 +0200
+++ popup.php   2008-08-17 16:49:06.000000000 +0200
@@ -33,7 +33,7 @@

        $REC_CRYPT_PASSWORD = (isset($amp_conf['AMPPLAYKEY']) && trim($amp_conf['AMPPLAYKEY']) != "")?trim($amp_conf['AMPPLAYKEY']):'moufdsuu3nma0';

-  $path = $crypt->decrypt($_REQUEST['recordingpath'],$REC_CRYPT_PASSWORD).$_REQUEST['recording'];
+  $path = $crypt->decrypt(urlencode($_REQUEST['recordingpath']),$REC_CRYPT_PASSWORD).$_REQUEST['recording'];

   // strip ".." from path for security
   $path = preg_replace('/\.\./','',$path);

I need a verification for this before I submit it to the SVN

08/17/08 12:46:28 changed by p_lindheimer

well it looks like we've both been working on this at the same time (after I saw your first post). I took a slightly different tact, maybe not quite the right one but here is also another problem.

First issue, as you point out, we urlencode, but the browser always urldecodes for you. So a few options exist:

  • what you did above although it's a little confusing and extra work encoding it before passing into the decrypt function)
  • Don't urldecode in the decrypt but keep urlencoding in the encrypt (what I did below but not symmetrical)
  • or don't urlencode in the encrypt and make the program using the encrypt method urlencode (symmetrical solution)

The other issue is that the filename component is never being urlencoded in the javascript. The following patch takes out the urldecode from the crypt function, and properly urlencodes the file name when sent to the popup window (I think/I hope):

Index: page.recordings.php
===================================================================
--- page.recordings.php (revision 6411)
+++ page.recordings.php (working copy)
@@ -421,7 +421,7 @@
        <!-- Begin
        function popUp(URL,optionId) {
                var selIndex=optionId.selectedIndex
-               var file=optionId.options[selIndex].value
+               var file=encodeURIComponent(optionId.options[selIndex].value)
 
                /*alert(selIndex);*/
                if (file != "")
Index: crypt.php
===================================================================
--- crypt.php   (revision 6411)
+++ crypt.php   (working copy)
@@ -62,7 +62,7 @@
    */
   function decrypt($enc, $salt, $iv_len = 16) {
 
-     $enc = base64_decode(urldecode($enc));
+     $enc = base64_decode($enc);
      $n = strlen($enc);
      $i = $iv_len;
      $str = '';

So the question, do we do this, or do we take out urlencoding all together in the crypt function and make the calling program responsible for that?

08/17/08 14:44:40 changed by mickecarlsson

My vote, after seeing your proposals (and testing it), is that we take out urlencoding in crypt and make the calling program doing the urlcoding.

08/17/08 14:48:46 changed by mickecarlsson

Clarifying: We take out urldecoding in crypt and make the calling program urlencode it.

08/17/08 15:02:23 changed by p_lindheimer

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [6417]) fixes #3058 really again, use encodeURIComponent() in javascript, and remove urlencoding from crypt function placing it in the calling functions so that crypt/encrypt is symetrical and not confusing

10/27/12 08:37:02 changed by sesekongkong