e107inc / e107

e107 Bootstrap CMS (Content Management System) v2 with PHP, MySQL, HTML5, jQuery and Twitter Bootstrap. Issue Discussion Room: https://gitter.im/e107inc/e107
https://e107.org
GNU General Public License v3.0
321 stars 214 forks source link

Downloads not working if I use the 'Local' tab to create the download: File Not Found Error #356

Closed jburns131 closed 11 years ago

jburns131 commented 11 years ago

When I created a download using the 'Local' tab and either select the file in the media manager, or upload the file using the media manager.

The download is created successfully, and displays fine, but when you click the download link it takes you to the next page and give a 'File Not Found' error.

The file path in the database started with {mediasomething}, don't remember quite what it was, but it looked accurate.

I had to uninstall/reinstall the Downloads plugin and recreate the download using the 'External' tab, using a directory on my web server.

As a side note, it would be nice to select 'None' when using the media manager to select files, because once you select one, you can't remove it. I'm not sure if it was user error, but once I selected a file in the media manager, it was there, and the 'External' download information didn't make the actual download button work.

In fact, the drop down for 'file' on the 'Local' tab continued to display the file I originally selected in the media manager, even after I use the media manager to remove the file from the system.

Moc commented 11 years ago

@jburns131 Can you test this issue again after applying the fix please? Thanks

Moc commented 11 years ago

Fixed, closing

jburns131 commented 11 years ago

The issue as I see it is that the wrong $_GET parameter format is being created when the /e107_plugins/download/download.php list and view pages are built.

If the file is in the old e107_files/downloads directory and not available to the media manager, the the $_GET parameter just contains the download ID which is stored in the downloads db table i.e. localhost/request.php?2

If the file is stored and recognized by the media manager, /request.php is expecting the format localhost/request.php?file=2, where the ID is the ID stored in the core_media db table, which does not correspond with the download table, especially if you are upgrading from a v1.x installation.

So when you click a download link from either the list or the view page, you are directed to /request.php. /request.php checks to see if $_GET['file'] exists. If so then use the media manager to provide the download. If there is no $_GET['file'] then use the legacy download method, which is /e107_plugins/download/request.php. It is the /e107_plugins/download/request.php that is giving the file not found error.

The downloads I am having trouble with are downloads that were uploaded and stored by the media manager, so it shouldn't even be processed by /e107_plugins/download/request.php in the first place. The proposed fix by Moc does work, but it's not addressing the real issue. The real issue is that the $_GET parameter format is incorrect when you reach /request.php.

The download links for the list page is being created separately from the download links on the view page, and both sets of links are passing the incorrect $_GET parameter format when a file is stored/available in the media manager.

It looks like those links are being built in e107_plugins/download/download.php. The list template is built starting at line 241, and the view template is built starting at line 349.

That's as far as I have gotten. I have been getting side tracked in my research as there are some subsystems that I'm not very familiar with (shortcodes and templates mainly), so as I find a piece of code I'm not familiar with I need to stop debugging, figure out how the subsystem works and fits into the overall picture, then go back to my debugging path.

I can continue to work on this if you'd like.

If I'm incorrect about this, please let me know. The largest indicator to me that this is the issue is that all downloads point to /request.php, and there are only two code branches in /request.php: If it's a media manager file use the new download method, if not use the legacy method.

CaMer0n commented 11 years ago

$_GET['file'] is used by the bbcode [file]media-file[/file] - it is not intended for use by the downloads plugin at all. If not for BC, file requests from the download plugin would only go through e107_plugins/downloads/request.php ?x, however, we are stuck with e_BASE.request.php?x for some time

e_BASE.request.php is not intended to be a central file for all plugins to utilize - on the contrary it should be only for core use such the core bbcodes and other needs which may arise in future.

The media-manager serves only one purpose for the downloads plugin - uploading and locating files when choosing them in its admin area.

Anyway, hope this clears things up. I believe this can be closed now, unless I've missed something?

jburns131 commented 11 years ago

Excellent, thanks for clearing that up. That was giving me a headache lol