Non-English characters are not converted correctly

[tverona1]

Hi Sophie,

I noticed that some non-English content end up not getting converted correctly. As a random example, try scraping this page: http://scp-wiki-cn.wikidot.com/scp-083

You'll see that the page is saved with a bunch of � characters.

It looks like the crux of the issue is that the content is always converted to "latin1" encoding.

The reason this is happening is that the request encoding is overridden to 'binary' (in config/defaults.js) but the responseType is the default of 'text'. So, when we get the request body, 'got' calls toString on its internal Buffer, passing in the 'binary' encoding, which is mapped to latin1 here:

https://github.com/nodejs/node/blob/fdce138e1dd86f63f141cbd5ec6b670eeec68986/lib/buffer.js#L708
if (encoding === 'latin1' || encoding === 'binary')
return encodingOps.latin1;

In order to address this, I get the response body as a Buffer (not text) and then later convert to utf8. Specifically, I'm doing the following:

1. In beforeRequest: Set request options to: requestOptions = {...requestOptions, encoding: 'binary', responseType: 'buffer'}
2. In afterResponse: If mime type is html / css / js, return reponse.body.toString('utf8'). Otherwise, return response.body (i.e. the Buffer object).
3. In request.js: In transformResult(), added an additional case:
   case Buffer.isBuffer(result):
   return {
   body: result,
   metadata: result.metadata || null
   };
4. In save-resource-to-fs-plugin.js, if type is html / css / js, set encoding to 'utf8' instead of 'binary'.

I haven't generalized this enough to submit a pull request but let me know if you need this.

[s0ph1e]

Hi @tverona1 👋

Thank you for submitting the issue, really interesting finding about latin1 encoding.

First of all - could you please check which version are you using? Would be nice to understand if there is a difference in using request vs using got for http calls.
Now latest published to npm version is 4.x (it uses request, not got) - it's in separate branch 4.x
Main branch contains work-in-progress for future version 5 which uses got which is not published to npm yet.

I'm open to review a PR if this encoding changes do not break other functionality. Would be super-nice if we can have a test which demonstrates a problem and code change which fixes it.

Do you think this changes should be applied by default?

[tverona1]

I'm using got, although request will behave similarly according to its documentation - since you're setting encoding to binary (aka latin1), both request and got will pass this as the encoding to Buffer's toString(). See this: https://nodejs.org/api/buffer.html#buffer_buffers_and_character_encodings

I think the desired default behavior for the scraper should be: Always get the body as a Buffer object (not text). When saving to file, save it as either binary or convert to utf8 according to its mime type. There's a pretty good library called mime-types with a function called mime.charset that will return the default charset of a content-type.

The above may be a breaking change, so should probably only be introduced in v5.

I don't have a ready made repro on the existing code base - I've actually forked it and changed it to use got a while back. I'll see if I can give it a try in the next couple of weeks. It should be easy to repro with a test case that has some non-latin content.

[tverona1]

Also, we should make sure there's a test case for scraping binary content (i.e. png) to make sure thesr changes don't regress that.

[s0ph1e]

Reopen because a version with a fix 5.1.0 was deprecated and reverted

[phawxby]

@s0ph1e do you have any suggestions on the best way to resolve this? We may need to use this plugin fairly soon to scrape a site which will suffer this issue so I can assign some engineering time to resolving the problem. However we're unfamiliar with the project so some guidance may be useful.

[s0ph1e]

Hi @phawxby

Thank you for your suggestion 👍

As a very quick solution you can use version 5.1.0, it's deprecated because it breaks functionality related to plugins, but if you don't need plugins you can give it a try

I don't have a clear vision how to implement a fix in a good way without breaking compatibility and/or major refactoring.

I'll try to have a closer look on your PR #496 this week