Upload class failing to upload file.

Need some custom code changes to the phpBB core simple enough that you feel doesn't require an extension? Then post your request here so that community members can provide some assistance.

NOTE: NO OFFICIAL SUPPORT IS PROVIDED IN THIS SUB-FORUM
Forum rules
READ: phpBB.com Board-Wide Rules and Regulations

NOTE: NO OFFICIAL SUPPORT IS PROVIDED IN THIS SUB-FORUM
JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Upload class failing to upload file.

Post by JH14 » Mon Sep 24, 2018 8:24 am

Hey,

I've tried using the upload class to upload an image, however, the image doesn't get moved to the server or even starts to upload.

I can't make sense of this:
https://area51.phpbb.com/docs/dev/3.2.x ... index.html#

Can anybody help, please?

Code: Select all

$picupload = (isset($_POST['picupload'])) ? true : false;
	if ($picupload == true) {
		$action = request_var('action', '');
		switch ($action)
		{
			case 'upload':
				//Our main code goes in here
				if (!class_exists('fileupload'))
				{
					include($phpbb_root_path . 'includes/functions_upload.' . $phpEx);
				}
				//Set upload directory
				$upload_dir = 'images/';
				//Upload file
				$upload = new fileupload();
				$upload->set_allowed_extensions(array('png', 'jpg', 'jpeg'));
				$file = $upload->form_upload('picupload');
				if (empty($file->filename))
				{
					trigger_error( 'File upload failed.' . adm_back_link($this->u_action), E_USER_WARNING);
				}
				$file->move_file($upload_dir, true);
				$gameimagenamefile = $file->filename;
				include("dbconnectfile.php");
				$addgame = "INSERT INTO indielix_gameimages (GameID, GameImage) VALUES ('$gameidnumber', '$gameimagenamefile')";

				if($conn->query($addgame)){
					$message = "Image added to game! $gameimagenamefile";
					writeSMes($message);
				}
				else{
					$message = "Error adding image to game!";
					writeEMes($message);
				}
				$conn->close();
			break;
		}		
	}

Here is the form:

Code: Select all

<div class="col-sm-2">
	<form action="" method="post" enctype="multipart/form-data" id="picupload" name="picupload">
		<input type="hidden" name="tempid" value="'.$row3['GameID'].'_'.$row3['GameName'].'"></input>
		<p>Select image to upload:</p>
		<input type="file" name="gameimgfileToUpload" id="gameimgfileToUpload"><br></br>
		<input type="submit" class="btn btn-primary" value="Upload Image" id="picuploaded" name="picuploaded">
	</form>
</div>
What am I doing wrong or are there other sections I need to include?

Any help would be greatly appreciated as I'm pulling my hair out over this.

Cheers

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Mon Sep 24, 2018 9:39 am

I see several issues:
- You can't use superglobals like $_POST and such
- You check for $_POST['picupload'] that isn't in the form
- request_var() is replaced by the $request class
- You use your own $conn class instead of injecting $dbal and your query is subject to SQL injection
- You switch only has 1 case so it's kinda useless.
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Tue Oct 02, 2018 11:16 am

Ger wrote:
Mon Sep 24, 2018 9:39 am
I see several issues:
- You can't use superglobals like $_POST and such
- You check for $_POST['picupload'] that isn't in the form
- request_var() is replaced by the $request class
- You use your own $conn class instead of injecting $dbal and your query is subject to SQL injection
- You switch only has 1 case so it's kinda useless.
So should it be more like this?

Code: Select all

	$picupload = (isset($_POST['picupload'])) ? true : false;
	if ($picupload == true) {
		$gametemp = request_var('tempid', '0');
		$gameidnumberarry =  explode("_", $gametemp);
		$gameidnumber = $gameidnumberarry[0];

		include($phpbb_root_path . 'includes/functions_upload.' . $phpEx);
		$upload_dir = 'images/';
		//Upload file
		$upload = new fileupload();
		$upload->set_allowed_extensions(array('png', 'jpg', 'jpeg'));
		$file = $upload->form_upload('gameimgfileToUpload');
		if (empty($file->filename))
		{
			trigger_error( 'File upload failed.' . adm_back_link($this->u_action), E_USER_WARNING);
		}
		$file->move_file($upload_dir, true);
		$gameimagenamefile = $file->filename;
		include("dbconnectfile.php");
		$addgame = "INSERT INTO indielix_gameimages (GameID, GameImage) VALUES ('$gameidnumber', '$gameimagenamefile')";

		if($conn->query($addgame)){
			$message = "Image added to game! $gameimagenamefile";
			writeSMes($message);
		}
		else{
			$message = "Error adding image to game!";
			writeEMes($message);
		}
		$conn->close();	
	}
I only need the basics working it's a prototype at the moment, the other parts will be sorted later, all I need it file upload working the rest will be dealt with later.

Cheers

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Tue Oct 02, 2018 11:43 am

It looks like most of my previous observations still stand.

NOFI, but it seems like you don't know the basics of phpBB (and have poor knowledge of writing secure PHP) so trying to help you with this fragment is kind of pointless. This should explain most of the file upload class. You can try to take a look at my cmBB extension where I handle file uploads as well. That is build using the extension system and responses are sent using the Symfony JSON response class.

Also: if you want help with this, you should provide some background information about your goals and setup and tell us what part works for you and what doesn't. Preferably with error messages if any.
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Wed Oct 03, 2018 2:45 am

Ger wrote:
Tue Oct 02, 2018 11:43 am
It looks like most of my previous observations still stand.

NOFI, but it seems like you don't know the basics of phpBB (and have poor knowledge of writing secure PHP) so trying to help you with this fragment is kind of pointless. This should explain most of the file upload class. You can try to take a look at my cmBB extension where I handle file uploads as well. That is build using the extension system and responses are sent using the Symfony JSON response class.

Also: if you want help with this, you should provide some background information about your goals and setup and tell us what part works for you and what doesn't. Preferably with error messages if any.
Thanks for this, I don't get any errors in the log or on the page. It's not for anything within the forum itself but a page outside of the forum on the main site where people will upload images to the system that are tied to data they have also added through a separate form earlier.

Mainly I'm very new to the way phpbb does stuff, for example I would use the normal way of uploading a file but too many errors occur because of the way phpbb deals with POST etc.

which part of it is insecure?

Cheers

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Wed Oct 03, 2018 9:09 am

JH14 wrote:
Wed Oct 03, 2018 2:45 am
which part of it is insecure?
SQL injection mainly.

But you do get errors you say. Usually phpbb error reporting is quite clear. Can you post those errors?
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Wed Oct 03, 2018 9:48 am

Ger wrote:
Wed Oct 03, 2018 9:09 am
JH14 wrote:
Wed Oct 03, 2018 2:45 am
which part of it is insecure?
SQL injection mainly.

But you do get errors you say. Usually phpbb error reporting is quite clear. Can you post those errors?
No sorry, I don't get any errors. All that happens is the page doesn't fully load after completion of the submit button. It loads the header which is above this segment then nothing beneath, but nothing in the error logs, unfortunately.


To prevent against injection attacks I normally use this:

Code: Select all

$NameHere = $conn->real_escape_string($VARIABLE_HERE);
Is this correct?

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Thu Oct 04, 2018 7:21 am

It might be, but it's usually the last resort. If you know something in your DB should be an integer, than just cast it as such:

Code: Select all

$gameidnumber = (int) $gameidnumber;
$gameimagenamefile = $db->sql_escape();
$query = "INSERT INTO indielix_gameimages (GameID, GameImage) VALUES ($gameidnumber, '$gameimagenamefile')";
Casting it as an integer will always result in a save query.

However: this is not the main point. Like I said, my previous obersvations still stand. I have pointed you to documentation as well as examples. You say you don't get any errors, however the page doesn't load fully. That means there is at least one error somewhere, probably suppressed. Is error reporting enabled on your server? Usually when you are developing, you need error reporting to be set to E_ALL.

Also, uncomment the debug lines in .config.php. That should give you some information to work with.
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Thu Oct 04, 2018 8:50 am

Ger wrote:
Thu Oct 04, 2018 7:21 am
It might be, but it's usually the last resort. If you know something in your DB should be an integer, than just cast it as such:

Code: Select all

$gameidnumber = (int) $gameidnumber;
$gameimagenamefile = $db->sql_escape();
$query = "INSERT INTO indielix_gameimages (GameID, GameImage) VALUES ($gameidnumber, '$gameimagenamefile')";
Casting it as an integer will always result in a save query.

However: this is not the main point. Like I said, my previous obersvations still stand. I have pointed you to documentation as well as examples. You say you don't get any errors, however the page doesn't load fully. That means there is at least one error somewhere, probably suppressed. Is error reporting enabled on your server? Usually when you are developing, you need error reporting to be set to E_ALL.

Also, uncomment the debug lines in .config.php. That should give you some information to work with.
Okay so, still no errors with it all on E_ALL.
EDIT:
E_ALL Is also enabled within cpanel under php settings

Is it possible to use this that you gave a link to? https://github.com/GerB/cmbb/blob/maste ... p#L68-L114

If so what parts do I need to change to get to work on my page? Also how would I get filename as a variable, nothing is obvious in the phpbb upload, filespec class docs

Disregarding the SQL at the moment as that can be worked on later. The primary thing I need to get working is file uploads.

Cheers

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Thu Oct 04, 2018 9:13 am

JH14 wrote:
Thu Oct 04, 2018 8:50 am
Okay so, still no errors with it all on E_ALL.
EDIT:
E_ALL Is also enabled within cpanel under php settings
Have you enabled the debug mode in config.php?
Is it possible to use this that you gave a link to? https://github.com/GerB/cmbb/blob/maste ... p#L68-L114
You can use it as an inspiration I guess. It's part of an extension and it uses dependency injection.
If so what parts do I need to change to get to work on my page? Also how would I get filename as a variable, nothing is obvious in the phpbb upload, filespec class docs
Well, many is actually quite clear. As you can see in my example, I use $file->clean_filename('real'); to clean any unwanted characters from the upload name and then $file->get('realname') to get the resulting name.


What you need to change... I guess it's better to start over.

Start with injecting the proper classes, that would at least be
\phpbb\request\request_interface $request
and
\phpbb\files\factory $files_factory

Next, initialise your upload with some configs:

Code: Select all

$upload = $this->files_factory->get('upload')
		->set_disallowed_content((isset($this->config['mime_triggers']) ? explode('|', $this->config['mime_triggers']) : false))
		->set_allowed_extensions($this->cmbb->allowed_extensions)
		->set_max_filesize($this->config['max_filesize']);
Change/expand this as to your liking


Handle the upload with

Code: Select all

$file = $upload->handle_upload('files.types.form', 'upload_form_field_name');
That should get you started.
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Thu Oct 04, 2018 11:01 am

Thank you.

This is what I now have:

Code: Select all


$factory = $phpbb_container->get('files.factory');
$upload = $phpbb_container->get('files.upload');
$filespec = $phpbb_container->get('files.filespec');
// We have a separate folder for each user. Let's make sure we have it.
$full_upload_dir = 'images';
if (!is_dir($full_upload_dir))
{
	mkdir($full_upload_dir, 0755, true);
}
$upload = $factory->get('upload')
		->set_allowed_extensions(array('png', 'jpg'))
		->set_error_prefix('ADDGAME_UPLOAD');
$file = $upload->handle_upload('files.types.form', 'featuredimage');
$file->clean_filename('real');
if (file_exists($full_upload_dir . '/' . $file->get('realname')))
{
	for ($i = 1; $i < 10; $i++)
	{
		$file->clean_filename('real', '1_');
		if (!file_exists($full_upload_dir . '/' . $file->get('realname')))
		{
			$approved = true;
			break;
		}
	}
	if (!isset($approved))
	{
		$file->clean_filename('real', date('YmdHis'));
	}
}
$file->move_file($full_upload_dir);
if (sizeof($file->error))
{
	$file->remove();
	echo "FAIL!";
}
else
{
	echo "WORKED!";
}
$featuredfilename = $file->get('realname');

Once this part starts processing, it echo's out "FAIL!" from that if (sizeof($file->error)) section.

Not sure what is causing that, any idea? Also at the end, the filename is still the same so $featuredfilename will be the name of the file before upload, so it doesn't run the clean on the name it seems.

Cheers

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Thu Oct 04, 2018 11:08 am

I'd start with

Code: Select all

var_dump($file->error);

To get an idea of what's wrong.
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Thu Oct 04, 2018 11:10 am

Ger wrote:
Thu Oct 04, 2018 11:08 am
I'd start with

Code: Select all

var_dump($file->error);

To get an idea of what's wrong.
array(1) { [0]=> string(34) "ADDGAME_UPLOADGENERAL_UPLOAD_ERROR" }

User avatar
Ger
Recognised Extension Developer
Posts: 1792
Joined: Wed Jan 02, 2008 7:35 pm
Location: 192.168.1.100
Contact:

Re: Upload class failing to upload file.

Post by Ger » Thu Oct 04, 2018 12:29 pm

There are 4 instances of this general upload error. They are all in the filespec->move_file() method. It's very straightforward to get the exact location where this error occurs by appending the error string with a simple "_test1", "_test2", etc.
My extensions:
Simple CMS, Feed post bot, Avatar Resize, Modbreak, Magic OGP, Live topic update, Modern Quote, Quoted Where (GDPR) and Autoresponder.
Newest: FAQ manager for 3.2

Like my work? Buy me a coffee to keep it coming. :ugeek:
-Available for custom work-

JH14
Registered User
Posts: 17
Joined: Thu Aug 09, 2018 12:14 pm

Re: Upload class failing to upload file.

Post by JH14 » Fri Oct 05, 2018 12:40 am

Hi,

After some tests, I deduced that the error was forming at the first clean_filename. The system started working when I changed it from 'real' to 'unique_ext'.

Here is the code I have now and it works fine.

Code: Select all

$factory = $phpbb_container->get('files.factory');
$upload = $phpbb_container->get('files.upload');
$filespec = $phpbb_container->get('files.filespec');

$full_upload_dir = '../images';
if (!is_dir($full_upload_dir)){
	mkdir($full_upload_dir, 0755, true);
}
$upload = $factory->get('upload')
		->set_allowed_extensions(array('png', 'jpg'))
		->set_error_prefix('ADDGAME_UPLOAD ');
$file = $upload->handle_upload('files.types.form', 'featuredimage');

$file->clean_filename('unique_ext');

if (file_exists($full_upload_dir . '/' . $file->get('realname'))){
	for ($i = 1; $i < 10; $i++){
		$file->clean_filename('unique_ext');
		if (!file_exists($full_upload_dir . '/' . $file->get('realname'))){
			$approved = true;
			break;
		}
	}
	if (!isset($approved)){
		$imgstatus = false;
	}
}
$file->move_file($full_upload_dir);
if (sizeof($file->error)){
	$file->remove();
	$imgstatus = false;
}
else{
	$imgstatus = true;
	$featuredfilename = $file->get('realname');
}
Thank you very much for your help :)

Cheers,
JH14

Post Reply

Return to “phpBB Custom Coding”

Who is online

Users browsing this forum: No registered users and 5 guests