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

Error when saving an Xlsx file (with images) with the same name! #1911

Closed
DiogoMestre opened this issue Mar 8, 2021 · 5 comments · Fixed by #1922
Closed

Error when saving an Xlsx file (with images) with the same name! #1911

DiogoMestre opened this issue Mar 8, 2021 · 5 comments · Fixed by #1922

Comments

@DiogoMestre
Copy link

This is:

- [x] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

An xlsx file with images can be saved several times with the same name, without errors occurring.

What is the current behavior?

When we try to save a file with images, that already exists with the same name, an error occurs.

PHP Fatal error:  Uncaught PhpOffice\PhpSpreadsheet\Writer\Exception: File zip://C:\Users\diogo\Documents\PhpSpreadsheet\test.xlsx#xl/media/54b0c99f8d2c2a408c3a8abee180cd941.png does not exist in C:\Users\diogo\Documents\PhpSpreadsheet\vendor\phpoffice\phpspreadsheet\src\PhpSpreadsheet\Writer\Xlsx\ContentTypes.php:198
Stack trace:
#0 C:\Users\diogo\Documents\PhpSpreadsheet\vendor\phpoffice\phpspreadsheet\src\PhpSpreadsheet\Writer\Xlsx\ContentTypes.php(126): PhpOffice\PhpSpreadsheet\Writer\Xlsx\ContentTypes->getImageMimeType()
#1 C:\Users\diogo\Documents\PhpSpreadsheet\vendor\phpoffice\phpspreadsheet\src\PhpSpreadsheet\Writer\Xlsx.php(216): PhpOffice\PhpSpreadsheet\Writer\Xlsx\ContentTypes->writeContentTypes()
#2 C:\Users\diogo\Documents\PhpSpreadsheet\BugReport.php(30): PhpOffice\PhpSpreadsheet\Writer\Xlsx->save()
#3 {main}
  thrown in C:\Users\diogo\Documents\PhpSpreadsheet\vendor\phpoffice\phpspreadsheet\src\PhpSpreadsheet\Writer\Xlsx\ContentTypes.php on line 198

What are the steps to reproduce?

  1. Create an Xlsx file with images.
  2. Save this file.
  3. Load the file;
  4. Save it again with the same name.

Note: I'm using the same code used by @PowerKiKi in this comment #544 (comment). The unique difference is that I'm saving the second Xlsx file with the same name as the first file.

<?php
require 'vendor/autoload.php';

use PhpOffice\PhpSpreadsheet\IOFactory;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx;

// Create new Spreadsheet object
$spreadsheet = new Spreadsheet();

// Generate an image
$gdImage = imagecreatetruecolor(250, 30);
$textColor = imagecolorallocate($gdImage, 255, 255, 255);
imagestring($gdImage, 4, 5, 5, 'Created with PhpSpreadsheet', $textColor);

// Add a drawing to the worksheet
$drawing = new MemoryDrawing();
$drawing->setImageResource($gdImage);
$drawing->setWorksheet($spreadsheet->getActiveSheet());

$writer = new Xlsx($spreadsheet);
$writer->save('test.xlsx');

$reloaded = IOFactory::load('test.xlsx');

$reloaded->getActiveSheet()->setCellValue('D5', 'foo');

$writer = new Xlsx($reloaded);
$writer->save('test.xlsx');

Which versions of PhpSpreadsheet and PHP are affected?

PhpSpreadsheet: v1.17.1
PHP: v7.4.13;

@luisfdgon
Copy link

I also stepped into this issue.

My workaround:

  1. Duplicate original xlsx file to a temporary one.
  2. Perform changes on temporary one.
  3. Save changes to original.
  4. Remove duplicated file.

This is an hackish solution and it brings some overhead, leading to performance issues in mass operations.

Hope this can be fixed soon.

@ccrims0n
Copy link
Contributor

ccrims0n commented Mar 14, 2021

This is specifically happening because of the way the xlsx file is opened before updating. It is opened in wb+ mode. Changing mode to wb doesn't throw the error as the Zip file can be read by another stream, however, other implications for mode are not known right now.

Although PhpOffice\PhpSpreadsheet\Writer\Xlsx\ContentTypes::getImageMimeType() tries to open the archive in default mode, i.e. ZipArchive::RDONLY, but it returns error 19 (ZipArchive::ER_NOZIP), hence it fails and returns the error File $pFile does not exist.

Ideally, the excel file shouldn't be modified unless all the data is read completely in memory. However, the current code overrides files in an incremental fashion rather than doing it once.

The solution suggested by @luisfdgon would work for this since the file from where the data is read and written are different.

I will try to file a good solution to this to fix but doesn't look file it'd be easy without a refactor. If any of the core contributors can confirm if that's okay, I'd be happy to file a PR.

@ccrims0n ccrims0n mentioned this issue Mar 14, 2021
5 tasks
@DiogoMestre
Copy link
Author

The solution/workaround suggested by @luisfdgon works fine for me! Thank you for that!

@ccrims0n, thank you for your detailed feedback about the problem and good luck with the work to find the best solution. I don't know if it is relevant or useful, but in PHPExcel this does not happen.

@ccrims0n
Copy link
Contributor

I don't know if it is relevant or useful, but in PHPExcel this does not happen.

The solution/workaround suggested by @luisfdgon works fine for me! Thank you for that!

@ccrims0n, thank you for your detailed feedback about the problem and good luck with the work to find the best solution. I don't know if it is relevant or useful, but in PHPExcel this does not happen.

That's strange, given both PHPExcel and PHPSpreadsheet have almost common code for this.
I'll debug more on this.

@MAKS-dev
Copy link

Perhaps he saves to xls with PHPExcel, which is a whole other file format (not a rar).

Anyway, may I be so bold to bump issue #1153 ? I'd really appreciate if someone would check that one as I'm hoping for a fix for so long (starting June 2018).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants