Check encode type for doanwload task #269
Conversation
|
|
|
||
| expectRequest('https://example.com').andRespond( | ||
| 200, | ||
| 'Hi', |
There was a problem hiding this comment.
This test will pass even if we haven’t encoding feature.
What do you think if we will use CP1251 and put that broken (when CP1251 was parsed as UTF-8) encoding text here?
There was a problem hiding this comment.
If it is too long review, I can do it myself
There was a problem hiding this comment.
hm, if i just cope paste broken text (like �), it doesnt work correctly. Continue research
There was a problem hiding this comment.
Another way is to get JS code for UTF-8→CP1251 convertation and put:
to1251('тест')There was a problem hiding this comment.
sorry, what do you mean "Try binary format for symbol"?
There was a problem hiding this comment.
sorry, what do you mean "Try binary format for symbol"?
core/download.ts
Outdated
| function detectEncodeType(response: Partial<Response>): string { | ||
| let headers = response.headers ?? new Headers() | ||
| let contentType = headers.get('content-type')?.toLowerCase() ?? '' | ||
| return contentType.match(/charset=([a-zA-Z0-9-]+)/)?.[1] ?? 'utf-8' |
There was a problem hiding this comment.
| return contentType.match(/charset=([a-zA-Z0-9-]+)/)?.[1] ?? 'utf-8' | |
| return contentType.match(/charset=(\w+)/)?.[1] ?? 'utf-8' |
Can we do like this?
There was a problem hiding this comment.
i think, no. in this case we get utf instead of utf-8
There was a problem hiding this comment.
We also need _
Here is the full list of variants
https://www.iana.org/assignments/character-sets/character-sets.xhtml
There was a problem hiding this comment.
maybe better include all symbols except space? something like that?
.match(/charset=([^\s]+)/)
There was a problem hiding this comment.
.match(/;\s*charset=([^\s;]+)/)
Technically it could be like Content-Type: text/html;charset=utf-8;boundary=ExampleBoundaryString
Fixes #268