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

UTF-8 BOM is not removed when running on Node.js and input is a file #1344

Closed
mojavelinux opened this issue Jul 2, 2021 · 4 comments · Fixed by #1345
Closed

UTF-8 BOM is not removed when running on Node.js and input is a file #1344

mojavelinux opened this issue Jul 2, 2021 · 4 comments · Fixed by #1345
Assignees

Comments

@mojavelinux
Copy link
Member

Although there's a test that covers removing the UTF-8 BOM from the AsciiDoc source, this only works on Node.js when the input is a string created in JavaScript. When the AsciiDoc source is read from a file, or the string is created from Buffer.from, the UTF-8 BOM is not removed.

This snippet explains why we're hitting this problem:

const fs = require('fs')

fs.writeFileSync('test.adoc', Buffer.concat([Buffer.from([0xEF, 0xBB, 0xBF]), Buffer.from('= Document Title')]))

const contentsFromFile = fs.readFileSync('test.adoc')
console.log(contentsFromFile.toString().charCodeAt())
console.log(contentsFromFile.toString().charCodeAt(1))
// => 65279
// => '='

const contentsFromString = '\xef\xbb\xbf= Document Title'
console.log(contentsFromString.charCodeAt())
console.log(contentsFromString.charCodeAt(3))
// => 239
// => '='

It appears that when Node.js creates a string by way of a Buffer, such as when reading the contents of a file, it changes the UTF-8 BOM into a different BOM character (code: 65279, char ref: 0xFEFF). I have not found any way to disable this behavior. It's basically a quirk of Node.js.

I think Asciidoctor.js should detect this alternate BOM and remove the character. (I'm open to changing Asciidoctor Ruby, if we determine it's necessary).

@mojavelinux
Copy link
Member Author

mojavelinux commented Jul 2, 2021

What I can't figure out is how to emulate this scenario in Ruby. I can't figure how to get Ruby to report the character code (8-bit unsigned integer) 65279. If I try to pack it, this is what I get:

[65279].pack('C*')
# => "\xFF"

Here's what I get if I unpack the BOM:

"\xFE\xFF".unpack('C*')
=> [254, 255]

And if I write it to a file, Ruby always writes the original BOM, "\xEF\xBB\xFB"

File.write '/tmp/bom.txt', [65279].pack('U*'), mode: 'wb'

We may just have to accept this character code is a weird quirk of Node.js and deal with it as such. There are translations going on we just can't see.

@mojavelinux
Copy link
Member Author

I'm now convinced it's best to deal with this in Asciidoctor.js. Our custom unpack method is an ideal place for it. Here's the logic we could use that would make this work:

`self.charCodeAt() === 65279 ? [239, 187, 191] : #{self[0, 3].bytes.select.with_index {|_, i| i.even? }}`

Let me know if you want me to proceed with a fix.

@mojavelinux
Copy link
Member Author

An alternate way to do this to check if self[0, 1].bytes is [255, 254]. Either way seems to work.

@mojavelinux
Copy link
Member Author

mojavelinux commented Jul 2, 2021

Here's something that's interesting:

encodeURIComponent(self.charAt())
// => %EF%BB%BF

and

encodeURIComponent(String.fromCharCode(65279))
// => %EF%BB%BF

So it is recognizing 65279 as a UTF-8 BOM.

I think checking the char code is the right strategy (as proposed above). It's the same as this, with a bit less overhead:

self.charAt() === String.fromCharCode(65279)

mojavelinux added a commit to mojavelinux/asciidoctor.js that referenced this issue Jul 3, 2021
mojavelinux added a commit to mojavelinux/asciidoctor.js that referenced this issue Jul 4, 2021
@mojavelinux mojavelinux self-assigned this Jul 4, 2021
ggrossetie pushed a commit to ggrossetie/asciidoctor.js that referenced this issue Jul 10, 2021
Bypass Ruby method calls to make BOM detection more portable.

Port asciidoctor#1344 fix.
ggrossetie pushed a commit to ggrossetie/asciidoctor.js that referenced this issue Jul 10, 2021
Bypass Ruby method calls to make BOM detection more portable.

Port asciidoctor#1344 fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant