pdffillerjs / pdffiller

Take an existing PDF Form and data and PDF Filler will create a new PDF with all given fields populated.
MIT License
289 stars 114 forks source link

Bug with utf8-fdf-generator #43

Open jbecwar opened 7 years ago

jbecwar commented 7 years ago

Hi All, Thanks so much for the lib. It saved me so much time.

A customer discovered that if they entered a ( or ) in a text field that would be saved to a pdf, then it would crash. We traced the bug back to this library and finally back to the utf8-fdf-generator lib.

The problem is that in the FDF spec (, ) and \ need to be escaped with a . That's a simple enough change, but the utf8-fdf-generator was converting the values to UTF-16 and then back to UTF-8 on file save. This ended up creating corrupted FDF files.

Long story short, it was easier for me to port in the generator lib and fix it here since this project has unit tests and they are both MIT license. I put in a pull request for the generator lib, but it hasn't been updated in 4 years and the author isn't on github much, so I don't know how long it will be before its pulled.

I also updated the libs and modified the unit tests to show the bug and the fixes.

If you don't want to merge this, I totally understand since it's really an upstream problem. I thought it would be helpful to anyone else that runs across this issue.

Thanks again, James

whitef0x0 commented 7 years ago

@jbecwar maybe easier to just include your forked library rather than changing the code of this one?

jasonphillips commented 7 years ago

I'd also like to see this merged. Including the fork as suggested sounds fine, but that means @jbecwar would first need to publish his fork on npm before this PR can be fixed up.

whitef0x0 commented 7 years ago

@jbecwar have you tried opening a pr for the utf library?

jasonphillips commented 7 years ago

@whitef0x0

He opened it here a couple of months ago: https://github.com/Rhaseven7h/utf8-fdf-generator/pull/2

Although, as he mentioned, the author of that library doesn't seem to be very active.

whitef0x0 commented 7 years ago

@jasonphilips @johntayl maybe this would be the time to transfer pdffiller and this patched utf8 library to a pdffiller organization

johntayl commented 7 years ago

@whitef0x0 @jasonphillips Copying this repo over to https://github.com/pdffillerjs

Will have to deprecate this one.

whitef0x0 commented 7 years ago

@johntayl if you transfer the repo you'll be able to keep all the stars and it'll have a redirect to it

ealtuna commented 5 years ago

Any updates over this issue?