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

Xlsx Reader Defined Name on Sheet with Apostrophe in Title #4360

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

oleibman
Copy link
Collaborator

@oleibman oleibman commented Feb 12, 2025

Fix #4356. Xlsx Reader needs to handle apostrophe for sheet title in defined name by converting doubled apostrophes to single.

Fix #4362. A similar problem to 4356, Style not handling sheet name with embedded apostrophe properly. And, with two examples in hand, I was able to determine a pattern to find and fix other possible exposures. It would not surprise me if there were similar exposures that I have not found yet. It also wouldn't be a huge surprise if this broke something, but no changes were required to the unit test suite to accommodate this change.

This is:

  • a bugfix
  • a new feature
  • refactoring
  • additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Fix PHPOffice#4356. Xlsx Reader needs to handle apostrophe for sheet title in defined name by converting doubled apostrophes to single.
@oleibman
Copy link
Collaborator Author

It looks like there are several areas that might be subject to the same problem. I will try to identify and remediate them in the next few days.

Fix PHPOffice#4362. A similar problem to 4360, Style not handling sheet name with embedded apostrophe properly. And, with two examples in hand, I was able to determine a pattern to find and fix other possible exposures.
@oleibman
Copy link
Collaborator Author

Scrutinizer's new issue is a false positive, and is now suppressed.

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