Skip to content

Conversation

@AbdelrahmanHafez
Copy link
Contributor

fixes #2762

I'm not if the modified files are source files that get compiled or those are the compiled files.
Also, can you point me to where should the test cases for those be?

it('handles base64 within data URI scheme (gh-2762), function() {
  // Arrange
  const fileInBase64 = 'TmFtZXMNCkhhZmV6DQpTYW0NCg==';
  const fileInBase64WithDataURIScheme = 'data:text/csv;base64,TmFtZXMNCkhhZmV6DQpTYW0NCg==';

  // Act
  const workBookFromRawBase64 = XLSX.read(fileInBase64, { type: 'base64' });
  const workBookFromBase64WithinDataURI = XLSX.read(fileInBase64WithDataURIScheme, { type: 'base64' });

  // Assert
  assert.deepStrictEqual(workBookFromRawBase64, workBookFromBase64WithinDataURI);
});

@SheetJSDev
Copy link
Contributor

The test should be added between line 690 and 691 in test.js: https://github.com/SheetJS/sheetjs/blob/master/test.js#L690-L691 (just after the "should read base64 strings" test)

The media type is optional and the comma need not be escaped. Try /^data:([^\/]+\/[^\/]+)?;base64,/

Include a test with a string like

  var fileInBase64WithDataURISchemeNoMediaType = 'data:;base64,TmFtZXMNCkhhZmV6DQpTYW0NCg==';

The test snippet should use avoid const and any other ES6+ features.

@AbdelrahmanHafez
Copy link
Contributor Author

@SheetJSDev
Thanks a lot for the detailed comment, I've pushed the necessary changes.

@SheetJSDev SheetJSDev merged commit 6bea47a into SheetJS:master Aug 8, 2022
@AbdelrahmanHafez AbdelrahmanHafez deleted the gh-2762 branch August 8, 2022 20:52
AbdelrahmanHafez added a commit to AbdelrahmanHafez/sheetjs that referenced this pull request Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] reading excel file from base64 should allow ignoring data uri information

2 participants