Skip to content

handle non-utf8 html document and their URLs#370

Merged
NGTmeaty merged 22 commits into
mainfrom
non-utf8-html
Aug 1, 2025
Merged

handle non-utf8 html document and their URLs#370
NGTmeaty merged 22 commits into
mainfrom
non-utf8-html

Conversation

@yzqzss

@yzqzss yzqzss commented Jun 27, 2025

Copy link
Copy Markdown
Collaborator

close #169

Some temporary references:


HTML:

html encoding:
https://encoding.spec.whatwg.org (spec, en)
https://html.spec.whatwg.org/multipage/parsing.html#determining-the-character-encoding (spec, en)
https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-charset (spec, en)
https://en.wikipedia.org/wiki/Windows-1252 (wiki, en)

websites declared ISO 8859-1 which is treated as Windows-1252 by all modern browsers (as required by the HTML5 standard) → https://w3techs.com/technologies/overview/character_encoding

https://www.xiaogd.net/md/web-page-encoding-and-mojibake-1 (blog, zh)
https://httptoolkit.com/blog/http-encodings/ (blog, en)


URL:

https://html.spec.whatwg.org/multipage/urls-and-fetching.html#resolving-urls

Let encoding be UTF-8.
If environment is a Document object, then set [encoding] (document charset) to environment's character encoding.

https://url.spec.whatwg.org/#query-state

  1. Percent-encode after encoding, with [encoding], buffer, and queryPercentEncodeSet, and append the result to url’s query.

https://url.spec.whatwg.org/#path-state

  1. ...special cases...
  2. Otherwise, run these steps:
    .... special cases ...
    3. UTF-8 percent-encode c using the path percent-encode set and append the result to buffer.

@yzqzss yzqzss self-assigned this Jun 27, 2025
@yzqzss yzqzss changed the title WIP: handle non-utf8-html html document and their URLs WIP: handle non-utf8 html document and their URLs Jun 27, 2025
@yzqzss yzqzss force-pushed the non-utf8-html branch 3 times, most recently from 9556b70 to 1d564da Compare July 14, 2025 14:22
@codecov-commenter

codecov-commenter commented Jul 26, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.83240% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.21%. Comparing base (62be009) to head (f9a1f66).
⚠️ Report is 84 commits behind head on main.

Files with missing lines Patch % Lines
...ernal/pkg/postprocessor/extractor/html_document.go 70.00% 21 Missing and 6 partials ⚠️
e2e/test/nonutf8encoding/nonutf8encoding.go 54.00% 18 Missing and 5 partials ⚠️
internal/pkg/postprocessor/extractor/html.go 90.90% 1 Missing and 1 partial ⚠️
pkg/models/item.go 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
+ Coverage   51.66%   55.21%   +3.55%     
==========================================
  Files         115      117       +2     
  Lines        7131     7257     +126     
==========================================
+ Hits         3684     4007     +323     
+ Misses       3154     2930     -224     
- Partials      293      320      +27     
Flag Coverage Δ
e2etests 37.63% <62.01%> (+8.34%) ⬆️
unittests 31.13% <65.89%> (+0.82%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yzqzss yzqzss marked this pull request as ready for review July 29, 2025 15:36
@yzqzss yzqzss changed the title WIP: handle non-utf8 html document and their URLs handle non-utf8 html document and their URLs Jul 29, 2025
@yzqzss yzqzss marked this pull request as draft July 29, 2025 15:43
@yzqzss yzqzss marked this pull request as ready for review July 29, 2025 16:06
@yzqzss yzqzss requested a review from Copilot July 29, 2025 20:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements support for handling non-UTF-8 HTML documents and their URLs by adding character encoding detection and proper URL encoding. It addresses the challenge of correctly processing web content that uses legacy character encodings like GBK, Shift_JIS, or Windows-1252.

  • Adds character encoding detection for HTML documents using golang.org/x/net/html/charset
  • Implements proper URL query parameter encoding based on document character encoding
  • Refactors HTML processing to use a new document transformation pipeline

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/models/url.go Adds document caching and encoding fields to URL struct with corresponding getters/setters
pkg/models/item.go Updates method call to use new document cache API
internal/pkg/postprocessor/item.go Adds document cache cleanup in post-processing
internal/pkg/postprocessor/extractor/html_document.go New file implementing character encoding detection and URL encoding logic
internal/pkg/postprocessor/extractor/html_document_test.go Comprehensive test coverage for encoding detection and URL encoding functions
internal/pkg/postprocessor/extractor/html.go Updates HTML extraction to use new document transformation and refactors asset extraction code
e2e/test/nonutf8encoding/ End-to-end test files for validating GBK encoding handling
Comments suppressed due to low confidence (1)

e2e/test/nonutf8encoding/testdata/gbk.html:4

  • The test data file contains Chinese characters that appear to be displayed incorrectly (showing as question marks or boxes). This suggests the file may not be saved in GBK encoding as intended, which could affect the validity of the test.
        <title>GBK �����ҳ��</title>

Comment thread pkg/models/url.go Outdated
Comment thread internal/pkg/postprocessor/extractor/html_document.go Outdated
@yzqzss yzqzss requested a review from NGTmeaty July 29, 2025 23:25

@NGTmeaty NGTmeaty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of small comments otherwise looks great!

Comment thread internal/pkg/postprocessor/extractor/html_document.go Outdated
Comment thread internal/pkg/postprocessor/extractor/html_document.go Outdated
Comment thread e2e/test/nonutf8encoding/nonutf8encoding_test.go Outdated
Comment thread e2e/test/nonutf8encoding/nonutf8encoding_test.go
Comment thread e2e/test/nonutf8encoding/nonutf8encoding.go Fixed
Comment thread e2e/test/nonutf8encoding/nonutf8encoding.go Fixed

@NGTmeaty NGTmeaty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you for the changes and tests!

@NGTmeaty NGTmeaty merged commit 9ef20b8 into main Aug 1, 2025
5 checks passed
@NGTmeaty NGTmeaty deleted the non-utf8-html branch August 1, 2025 22:25
@yzqzss yzqzss added the GSoC label Aug 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle non-UTF8 HTML pages

5 participants