Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JFile::copy to check realpath #18352

Closed
stAn47 opened this issue Oct 16, 2017 · 11 comments
Closed

JFile::copy to check realpath #18352

stAn47 opened this issue Oct 16, 2017 · 11 comments

Comments

@stAn47
Copy link

stAn47 commented Oct 16, 2017

Steps to reproduce the issue

Hello friends, it would be nice if JFile::copy checked if the src and dest paths are the same. Right now it gives an error and thus installer or other dependencies fail upon such a case.

Usage:
during the development we are using an installer path in an component directory such as /administrator/components/com_test/install/systempluginname

which is symlinked to
/plugins/system/systempluginname

upon changes in the component BE config, it either enables or disables particular plugins that are used per the component's logic, but mainly it makes sure that the plugins is up-to-date by updating it with the Joomla installer.

it would be nice if JFile::copy did not return an error upon same src and dest especially if those directories are symlinked and are seen as the same by "realpath".

i do not really know how to do a PR, but i would suggest:

$src = realpath($src); 
$dest = realpath($dest); 
if ($src === $dest) return true; 

into:
\libraries\joomla\filesystem\file.php

on Joomla 3.8.1

at about line 99

full function would then look like:

/**
	 * Copies a file
	 *
	 * @param   string   $src          The path to the source file
	 * @param   string   $dest         The path to the destination file
	 * @param   string   $path         An optional base path to prefix to the file names
	 * @param   boolean  $use_streams  True to use streams
	 *
	 * @return  boolean  True on success
	 *
	 * @since   11.1
	 */
	public static function copy($src, $dest, $path = null, $use_streams = false)
	{
		
		
		$pathObject = new JFilesystemWrapperPath;

		// Prepend a base path if it exists
		if ($path)
		{
			$src = $pathObject->clean($path . '/' . $src);
			$dest = $pathObject->clean($path . '/' . $dest);
			
		}
		
		$src = realpath($src); 
		$dest = realpath($dest); 
		if ($src === $dest) return true; 
		
		// Check src path
		if (!is_readable($src))
		{
			JLog::add(JText::sprintf('JLIB_FILESYSTEM_ERROR_JFILE_FIND_COPY', $src), JLog::WARNING, 'jerror');

			return false;
		}

		if ($use_streams)
		{
			$stream = JFactory::getStream();

			if (!$stream->copy($src, $dest))
			{
				JLog::add(JText::sprintf('JLIB_FILESYSTEM_ERROR_JFILE_STREAMS', $src, $dest, $stream->getError()), JLog::WARNING, 'jerror');

				return false;
			}

			return true;
		}
		else
		{
			$FTPOptions = JClientHelper::getCredentials('ftp');

			if ($FTPOptions['enabled'] == 1)
			{
				// Connect the FTP client
				$ftp = JClientFtp::getInstance($FTPOptions['host'], $FTPOptions['port'], array(), $FTPOptions['user'], $FTPOptions['pass']);

				// If the parent folder doesn't exist we must create it
				if (!file_exists(dirname($dest)))
				{
					$folderObject = new JFilesystemWrapperFolder;
					$folderObject->create(dirname($dest));
				}

				// Translate the destination path for the FTP account
				$dest = $pathObject->clean(str_replace(JPATH_ROOT, $FTPOptions['root'], $dest), '/');

				if (!$ftp->store($src, $dest))
				{
					// FTP connector throws an error
					return false;
				}

				$ret = true;
			}
			else
			{
				if (!@ copy($src, $dest))
				{
					JLog::add(JText::sprintf('JLIB_FILESYSTEM_ERROR_COPY_FAILED_ERR01', $src, $dest), JLog::WARNING, 'jerror');

					return false;
				}

				$ret = true;
			}

			return $ret;
		}
	}

Expected result

Do not give an error upon same src and dest

Actual result

It now fails...

System information (as much as possible)

Additional comments

@ghost
Copy link

ghost commented Jan 26, 2018

Any Comment on this Feature Request?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18352.

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @alikon by The JTracker Application at issues.joomla.org/joomla-cms/18352

@alikon
Copy link
Contributor

alikon commented Jul 5, 2019

lack of interest


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18352.

@HLeithner
Copy link
Member

Fixing it would be better because it's a vaild issue.

@HLeithner HLeithner reopened this Jul 5, 2019
@mbabker
Copy link
Contributor

mbabker commented Jul 5, 2019

If you're going to close issues because "lack of interest", use a more communicative message than that. Remember this is a public platform and people do get email notifications.

@alikon
Copy link
Contributor

alikon commented Jul 5, 2019

what should be "a more communicative message" for an issue opened on 16 Oct 2017 without no response till i've closed it in 05 july 2019, sorry but i'm not an english native speaker, but nevermind,
i'll never close more an old issue , let they stay open forever for future memories, lesson learned

@brianteeman
Copy link
Contributor

Can I suggest using these messages that I wrote for the last sprint I organised
https://docs.google.com/document/d/1ODy85pPR6y6OHgyx8SRrZ93hR8JtsJKNyj5AOQrNlmA/edit?usp=sharing

They can even be added as saved replies on github

@alikon
Copy link
Contributor

alikon commented Jul 5, 2019

you should have shared this before, only option 2) and 11 and all was ok 🤣

@brianteeman
Copy link
Contributor

You already had it from the Manchester sprint :)

@mbabker
Copy link
Contributor

mbabker commented Jul 5, 2019

i'll never close more an old issue , let they stay open forever for future memories, lesson learned

Closing old items isn't an issue, if:

  • in the case of a feature request, someone decides the project isn't interested in including that request in core
  • in the case of a confirmed bug report, someone decides that behavior is actually the intended behavior or that doing something about it creates more issues than it solves (basically B/C problems)
  • in the case of an unconfirmed bug report, there isn't enough info to do anything with the item and there's no response to requests for more info

The only thing keeping items open does is raise the number in the tabs at the top of the GitHub repo. Trying to arbitrarily close issues that people have been able to confirm as bugs because nobody is interested enough in fixing those bugs doesn't really help matters. It may very well be those issues are hard to address or edge case issues (like a couple of the IE8 related issues, they're still valid since 3.x supports IE8 but they're more than likely going to stay open and unactioned until 3.x EOLs because few are going to care enough to put in the effort to fix compat issues with that old of a browser), but they're still valid.

@rdeutz
Copy link
Contributor

rdeutz commented Apr 29, 2024

closing here and moved issue to the framework: joomla-framework/filesystem#65

@rdeutz rdeutz closed this as completed Apr 29, 2024
@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Issue Cleanup Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

8 participants