• Resolved iporter

    (@iporter)


    Hi, not sure if this is the appropriate place to file a bug report – let me know if not.

    Line 386 of wp-smushit.php is ‘@rename( $temp_file, $file_path );’. Apparently this function doesn’t always work on Windows, and the result should be tested and if not successful, use copy()/unlink() instead – see https://php.net/manual/en/function.rename.php.

    A such, I’ve had to replace it with the following, but it took a while to figure out why smushit was reporting nothing but success yet the files weren’t getting any smaller!

    $success = @rename( $temp_file, $file_path );
    if (!$success) {
    	copy($temp_file, $file_path);
    	unlink($temp_file);
    }

    Thanks,
    Iain

    P.S. Also, more of an edge case but a bug nonetheless – the condition on line 321
    if ( 0 !== stripos(realpath($file_path), realpath(ABSPATH)) ) {
    doesn’t accommodate installations in which the core files are stored in a subdirectory (useful when working with svn externals – see https://ottopress.com/2011/creating-a-wordpress-site-using-svn/). I’ve had to change this to
    if ( 0 !== stripos(realpath($file_path), str_replace('/core', '', realpath(ABSPATH))) ) {
    for my needs, but this only works for my choice of directory name – perhaps you understand the required fix?

    Thanks.

    https://www.ads-software.com/plugins/wp-smushit/

Viewing 7 replies - 1 through 7 (of 7 total)
  • Mike

    (@michael-copestake)

    Hi @iporter

    Well spotted, fewer people use Windows servers which is why this probably hasn’t been identified until now.

    I’ve given our developers a heads up so hopefully they will implement this.

    Thank you very much for the in depth description and for providing a solution!

    @iporter,

    Thanks for the code snippet on the rename. I agree. Part of the issue with Windows is not being able to rename across partitions.

    also thanks for the notes on the code related to the str_replate logic. Ive done away with all that cruft. Basically, the plugins should spin through the media attachments and work under any path. These is some funky left over code that limits the image to be within certain paths which in my option is wrong. So now the plugin (in beta) takes the URL and Path of the original image and simply uses that.

    I don’t have access to a Windows system. Any chance of getting you to review the beta?

    Thread Starter iporter

    (@iporter)

    For sure, and you’re welcome – happy to be able to help. How do I download it, is it in SVN? I see v1.6.5, which is what I have installed.

    Mike

    (@michael-copestake)

    Hi @iporter

    The latest version has now been uploaded, its 1.6.5.1 and you can grab it here:
    https://downloads.www.ads-software.com/plugin/wp-smushit.latest-stable.zip

    Please report back and let us know how you get on as most of us are using Linux based servers and can’t test these things!

    Hey again.

    Thanks again, we’ve release that version as per Mikes post.

    If you need anything else, please let us know.

    Take care.

    Thread Starter iporter

    (@iporter)

    Hi Mike, apologies for the delay. I’ve just downloaded version 1.6.5.3 and can confirm that your updates have solved this problem.

    Cheers!

    Mike

    (@michael-copestake)

    Brilliant, thank you for letting us know!

Viewing 7 replies - 1 through 7 (of 7 total)
  • The topic ‘Bug Report: @rename on Windows’ is closed to new replies.