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

Docx is invalid if the written attributed string contains an ampersand (&) or less-than (<) character #18

Closed
andalman opened this issue Oct 6, 2022 · 4 comments

Comments

@andalman
Copy link
Contributor

andalman commented Oct 6, 2022

This is easy to repro using these simple tests:

    func testAmpersand() {
        let string="Key & Peele"
        let attributedString=NSAttributedString(string: string)

        testWriteDocX(attributedString: attributedString)
    }

    func testLessThan() {
        let string="0 < 1"
        let attributedString=NSAttributedString(string: string)

        testWriteDocX(attributedString: attributedString)
    }

If you then run these tests, they will fail with: [DocXTests.DocXTests testAmpersand] : failed - The file…couldn’t be opened because it isn’t in the correct format.

@andalman
Copy link
Contributor Author

andalman commented Oct 6, 2022

Replacing the & with &amp; and < with &lt; would fix this bug. Clients can do this on their end, but it seems better (and safer) to sanitize the string before writing it.

@andalman
Copy link
Contributor Author

andalman commented Oct 6, 2022

One more thing: the DocXOptions (e.g. author, title, etc.) should be sanitized as well. If those strings include ampersands and less-than symbols, then the docx will give a warning when it is opened in Word.

@andalman
Copy link
Contributor Author

andalman commented Oct 6, 2022

I have a fix, and will submit a pull request shortly.

andalman added a commit to oneeightyg/DocX that referenced this issue Oct 7, 2022
This will properly escape reserved characters that appear in the document and in the DocXOptions.

shinjukunian#18
@andalman andalman mentioned this issue Oct 7, 2022
@shinjukunian
Copy link
Owner

again , thank you for catching and fixing this issue.
In addition, the improved testing seems like a very good idea.

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

No branches or pull requests

2 participants